Community
Participate
Working Groups
Created attachment 183990 [details] JUnit I attached a JUnit test. After patch is applied the JUnit will be added to org.eclipse.draw2d.test plug-in. PrecisionRectangleTests class. The test is testScale. LaunchPrecisionRectangleTests as Plug-in JUnit test. Note that testScale fails. It would pass with Draw2D before the fix to Bug 271235 updatePreciseWidthDouble() method implementation is: if (width != PrecisionGeometry.doubleToInteger(preciseX() + preciseWidth) - x) { preciseWidth = width; } Do we really want the x to be present in the calculation? Wouldn't that affect the order of how we scale and/or set new width and height, i.e. before/after x and y are set? This is very important to fix, because we get infinite loops on opening some diagrams in our product.
Alex, you are right, updatePreciseWeight()and updatePreciseHeight() should be independent of x and y. And I think the same has to hold for updateWidthInt() and updateHeightInt(), which also take the x and y values into account. I introduced this, because the original updateInts() method (which is now deprecated) was implemented as: x = (int) Math.floor(preciseX + 0.000000001); y = (int) Math.floor(preciseY + 0.000000001); width = (int) Math.floor(preciseWidth + preciseX + 0.000000001) - x; height = (int) Math.floor(preciseHeight + preciseY + 0.000000001) - y; Here of course, all fields were always updated together and the "reverse" synchronization was not in place (this was introduced with 3.7).
I changed that updateWidthInt(), updateHeightInt(), updatePreciseWidthDouble() and updatePreciseHeightDouble() only use low and high precision field they have to update in their calculation. I also added your test case (which I augmented with assertions for x, y, and height) to Draw2d. Committed changes to cvs HEAD (3.7).
Changed verison to 3.7 from 3.6.2. Verified that we don't get infinite loops on opening diagrams. Thanks very much for the quick turn around on this :-)
(In reply to comment #3) > Changed verison to 3.7 from 3.6.2. Does this mean we need the fix in 3.6.2 as well?
(In reply to comment #4) > (In reply to comment #3) > > Changed verison to 3.7 from 3.6.2. > > Does this mean we need the fix in 3.6.2 as well? No, the refactoring of precision API was performed in the 3.7 stream only, so 3.6.2 is not affected.
