Bug 237802

Comment 1 Randy Hudson 2008-06-19 12:10:12 EDT

API can not be changed.  Will this work instead:
return new PrecisionPoint((a1.preciseX() + d1.preciseWidth())
	* (1f - weight) + weight * (a2.preciseX() + d2.preciseWidth()),
	(a1.preciseY() + d1.preciseHeight()) * (1f - weight) + weight
	* (a2.preciseY() + d2.preciseHeight()));

Change 1f to 1.0, so that the subtraction is done using double precision.

Comment 2 Stéphane Lizeray 2008-06-20 03:42:11 EDT

Well, for my use case, yes your suggestion fixes the bug.

Regarding changing the signature from float to double, the API will be backward compatible, so what is the problem ?

Because even it fixes my current problem, there is still a potential problem with GMF,

https://bugs.eclipse.org/bugs/show_bug.cgi?id=237803

org.eclipse.gmf.runtime.diagram.ui.editparts.ConnectionEditPart#refreshBendpoints

    protected void refreshBendpoints() {
        RelativeBendpoints bendpoints = (RelativeBendpoints) getEdge()
            .getBendpoints();
        List modelConstraint = bendpoints.getPoints();
        List figureConstraint = new ArrayList();
        for (int i = 0; i < modelConstraint.size(); i++) {
            org.eclipse.gmf.runtime.notation.datatype.RelativeBendpoint wbp = (org.eclipse.gmf.runtime.notation.datatype.RelativeBendpoint) modelConstraint
                .get(i);
            RelativeBendpoint rbp = new RelativeBendpoint(getConnectionFigure());
            rbp.setRelativeDimensions(new Dimension(wbp.getSourceX(), wbp
                .getSourceY()), new Dimension(wbp.getTargetX(), wbp
                .getTargetY()));
            if (modelConstraint.size() == 1) {
            	rbp.setWeight(0.5f);
            } else {
--->           	rbp.setWeight(i / ((float) modelConstraint.size() - 1));
            }
            figureConstraint.add(rbp);
        }
        getConnectionFigure().setRoutingConstraint(figureConstraint);
    }

Could you include your fix in Ganymede ? Thanks

Comment 3 Randy Hudson 2008-06-20 10:05:35 EDT

> Regarding changing the signature from float to double, the API will be backward
> compatible, so what is the problem ?

How is a signature change backwards compatible?  This would break both binary and source compatibility.

> Because even it fixes my current problem, there is still a potential problem
> with GMF,

I don't see the issue with the above fix. Weighting is just a hint about whether the bendpoint should stay closer to the source or target end of the connection.  It could be a number from 1-100, and things should still work about the same to the user, even if you had over 100 bendpoints.

Comment 4 Stéphane Lizeray 2008-06-20 10:32:55 EDT

> How is a signature change backwards compatible?  This would break both binary
> and source compatibility.


I was speaking about source compatibility. It will be source compatible because there is an automatic casting from float to double.

Anyway, your are right on the other part of your comment. You change will fix all the use cases.

Thanks

Comment 5 Stéphane Lizeray 2008-06-20 10:37:25 EDT

Let me know if you can include your fix in Ganymede? Thx

Comment 6 Anthony Hunter 2008-06-20 10:57:07 EDT

(In reply to comment #5)
> Let me know if you can include your fix in Ganymede? Thx
> 

Ganymede is closed but I think we can put this in the maintenance release.

Comment 7 Romain Raugi 2008-08-08 10:17:39 EDT

Created attachment 109526 [details]
Patch that contains Randy Hudson's fix.

Comment 8 Anthony Hunter 2008-08-08 11:01:49 EDT

Thanks for the patch.

Stephane, can you confirm this latest patch works for you?

Any ideas on how we can add a JUnit test that demonstrates the failure?

Comment 9 Romain Raugi 2008-08-11 09:11:09 EDT

This patch works for us. I will write a JUnit test to highlight the bug.

Comment 10 Romain Raugi 2008-08-18 09:02:32 EDT

Created attachment 110220 [details]
Screenshots, before and after having applied the fix.

Comment 11 Romain Raugi 2008-08-18 09:03:17 EDT

Created attachment 110221 [details]
Unit test to highlight the bug.

Comment 12 Anthony Hunter 2008-09-08 11:25:08 EDT

Committed to R3_4_maintenance and HEAD