Fix improper bounds calculation for MoveHandleLocator when zoomed in

[ptziegler]

The MoveHandleLocatorshrinks the size of the target figure by one, handles the insets and then expands it by one again. The problem is that the first operation is done before translating the bounds to absolute coordinates. Instead of shrinking the size by one, it is therefore shrunk by one times the zoom factor.

As a result, the bounds of the target figure don't fully enclose the bounds of the figure owned by the locator.

To reproduce: Open the Logic editor and select e.g. a Gate. When selecting the element, the selection box doesn't fully enclose the right and bottom of the figure.

[ptziegler]

For illustration, I've updated the Gate figure to draw a simple rectangle. As you can see, the selection box is too small:

The initial change was done with 7a79061 and e7a9fda. But due to the poor commit message, I can't say anything about why it was done like this.

[azoitl]

Could it be that the -1 +1 thingy somehow wants to address the line border width. However this should be handled by the insets or? Couldn't we remove the -1 +1?

[ptziegler]

Could it be that the -1 +1 thingy somehow wants to address the line border width. However this should be handled by the insets or? Couldn't we remove the -1 +1?

That's what I meant... If you look at how it was originally, the insets were taken into consideration. The +1/-1 was then added for the sake of some "zoom changes", but without any explanation what they're supposed to fix.

gef-classic/org.eclipse.gef/src/org/eclipse/gef/handles/MoveHandleLocator.java

Lines 43 to 54 in 3e73cf3

	public void relocate(IFigure target) {
	Insets insets = target.getInsets();
	Rectangle bounds;
	if (getReference() instanceof HandleBounds)
	bounds = ((HandleBounds)getReference()).getHandleBounds();
	else
	bounds = getReference().getBounds();
	bounds = bounds.getExpanded(insets.left, insets.top);
	getReference().translateToAbsolute(bounds);
	target.translateToRelative(bounds);
	target.setBounds(bounds);
	}

I don't see why it was added but I also can't predict the side-effects that would be introduced by removing them. My current guess is that it likely avoids some rounding errors when scaling the figures.

All I can say is that it not a +1/-1, but rather a +1/(-1*zoom), which is causing these artifacts.

[azoitl]

I don't see why it was added but I also can't predict the side-effects that would be introduced by removing them. My current guess is that it likely avoids some rounding errors when scaling the figures.

All I can say is that it not a +1/-1, but rather a +1/(-1*zoom), which is causing these artifacts.

Me neither. I only fear that now that you changed it to PrecisionRectangle that we have now more or different rounding issues and that the +1/(-1*zoom) adds to that. However I have no clue how to find that out.

[ptziegler]

I only fear that now that you changed it to PrecisionRectangle that we have now more or different rounding issues and that the +1/(-1*zoom) adds to that

I'm not sure what you mean. The class was using a PrecisionRectangle even before my change.

[azoitl]

I'm stupid.