Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 331280

Summary:

Precise Draw2D geometry is incompatible with the old one

Product:

[Tools] GEF

Reporter:

Alex Boyko <aboyko>

Component:

GEF-Legacy Draw2d

Assignee:

Alexander Nyßen <nyssen>

Status:

CLOSED FIXED

QA Contact:

Severity:

major

Priority:

CC:

ahunter.eclipse, nyssen

Version:

3.7

Target Milestone:

3.7.1 (Indigo) M4

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
JUnit	none

Description Alex Boyko

2010-11-28 02:07:13 EST

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.

Comment 1 Alexander Nyßen

2010-11-28 05:09:46 EST

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).

Comment 2 Alexander Nyßen

2010-11-28 05:10:00 EST

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).

Comment 3 Alex Boyko

2010-11-28 14:27:07 EST

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 :-)

Comment 4 Anthony Hunter

2010-11-29 09:24:24 EST

(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?

Comment 5 Alexander Nyßen

2010-11-29 09:42:03 EST

(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.