Bug 212280 - org.eclipse.draw2d.RelativeBendpoint#getLocation() returns a point with a slight error
Summary: org.eclipse.draw2d.RelativeBendpoint#getLocation() returns a point with a sli...
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.2.1   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4.0 (Ganymede) M6   Edit
Assignee: Alex Boyko CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-12-07 11:44 EST by Alex Boyko CLA
Modified: 2008-09-18 13:35 EDT (History)
4 users (show)

See Also:


Attachments
Patch for RelativeBendpoint (1.86 KB, application/octet-stream)
2007-12-07 11:47 EST, Alex Boyko CLA
no flags Details
patch (1.86 KB, patch)
2007-12-11 17:50 EST, Alex Boyko CLA
no flags Details | Diff
Uses internally PrecisionDimension (2.28 KB, patch)
2007-12-19 12:40 EST, Stéphane Lizeray CLA
ahunter.eclipse: iplog+
Details | Diff
final patch (2.39 KB, patch)
2008-02-25 17:08 EST, Alex Boyko CLA
ahunter.eclipse: iplog+
aboyko: review-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Boyko CLA 2007-12-07 11:44:11 EST
1. Create a wire between two logic elements on the diagram.
2. Drag the middle point of the wire to create a bend point with 100% zoom.
The bend point is moved normally and when the mouse button is released the wire stays exactly the same as at the moment before the button was released.
Now, change the zoom to 53% and perform the same operation to the bend point.
You should notice that the connection jumps a little bit when the mouse button is released.

Problem: dimensions stored by RelativeBendpoint are relative to the connection figure. When the calculation of a bend point location is done these dimensions are translated to absolute coordinates and, hence, with a non-integer scaling factor, a loss of precision occurs. Therefore, the point we get in absolute coordinates translated back to relative coordinates will be a bit off its real location.

Suggestion: create precision points from connection anchor reference points, translate them relative to connection. Perform the calculation to locate the bend point with weight and dimensions and return a PrecisionPoint for the location. This will reduce the slight precision error.

Patch to be attached
Comment 1 Alex Boyko CLA 2007-12-07 11:47:53 EST
Created attachment 84746 [details]
Patch for RelativeBendpoint

The patch for RelativeBendpoint with a fix described in the previous comment.
This issue affects GMF, because it has a concept of MapMode (HiMetrics) which introduces another non-integer scaling factor aside zoom.

Randy, Pratik is it ok for this fix to be committed?
Thanks in advance.
Comment 2 Alex Boyko CLA 2007-12-11 17:50:23 EST
Created attachment 85012 [details]
patch

Previous patch was not uploaded correctly. Here is the correctly uploaded patch. Sorry for any inconvenience.
Comment 3 Stéphane Lizeray CLA 2007-12-19 12:40:19 EST
Created attachment 85584 [details]
Uses internally PrecisionDimension

This patch work if the PrecisionDimension copy constructor has been fixed.
See https://bugs.eclipse.org/bugs/show_bug.cgi?id=213495


Uses internally PrecisionDimension
Comment 4 Alex Boyko CLA 2007-12-19 14:36:26 EST
Perhaps, we'd like to support PrecisionDimension in RelativeBendpoint? In this case, I could modify the visibility of preciseX(),...,preciseHeight() methods on Point, Dimension, Rectangle to public - the patch for bug 212460
Hence, the patch for this one will be modified accordingly.
Comment 5 Anthony Hunter CLA 2008-02-08 15:32:53 EST
Missed M5, so moving to M6
Comment 6 Anthony Hunter CLA 2008-02-25 16:19:14 EST
(In reply to comment #4)
> Hence, the patch for this one will be modified accordingly.

I think we still are waiting for an updated patch, right?

Comment 7 Alex Boyko CLA 2008-02-25 17:08:51 EST
Created attachment 90696 [details]
final patch

I've attached the revised pacth fixing the issue. It uses latest API additions to Point (i.e. preciseX(), preciseY() methods.
Rnady, could you please code review this fix and provide feedback?
Thanks.
Comment 8 Anthony Hunter CLA 2008-03-26 15:37:35 EDT
Do we have any concerns that the location for the RelativeBendpoint now returns a PrecisionPoint rather than a Point. (i.e. x and y are not float). 

I am not 100% sure we have called updateInts() when creating the PrecisionPoint.
Comment 9 Alex Boyko CLA 2008-03-26 15:53:55 EDT
PrecisionPoint(double, double) calls updateInts(), so we should be good in terms of integer values for Point.
Comment 10 Anthony Hunter CLA 2008-03-26 17:00:57 EDT
ok then, committed to HEAD