Use SourceFigure for ScrollableThumbnail handling

[azoitl]

The SourceFigure in the ScrollableThumbnail may be different then the viewport's content used. In that cases the size and position of the Selector was not handled correctly.

If the SourceFigure is the content of the viewport then it behaves as before.

@Phillipus @ptziegler what do you think. For me it behaves now more correct and opens a nice use case for 4diac IDE, where I otherwise would need to copy the whole class, which I found not as nice.

With all the automatic clean-ups, the interesting lines are: 200, 204, 235, & 236.

[github-actions]

Unit Test Results

    9 files      9 suites   19s ⏱️
307 tests 305 ✔️ 2 💤 0 ❌
921 runs  915 ✔️ 6 💤 0 ❌

Results for commit 24385a3.

♻️ This comment has been updated with latest results.

[Phillipus]

@azoitl

I tested it in Archi and it's not working properly if the diagram is zoomed less than or greater than 100% and a diagram object has negative co-ordinates (we use a FreeFormLayer that allows an object to have negative x,y). The selection area cannot be moved to the negative space in the thumbnail or moves outside of the thumbnail area.

For example in reconfigureSelectorBounds():

viewport.getHorizontalRangeModel().getMinimum() returns -123
getSourceRectangle().x returns -246

viewport.getVerticalRangeModel().getMinimum() returns -63
getSourceRectangle().y returns -126

Therefore this isn't going to work in this case.

[azoitl]

@Phillipus this is interesting I can not reproduce the behavior in 4diac IDE. I guess our incremental growing drawing canvas with the border is behaving differently. That means I have to duplicate the code for 4diac IDE, because I don't see a clear strategy in making the ScrollableThumbnail more adaptable.

[Phillipus]

@azoitl I'm seeing an incorrect size and position for the selection figure (the blue rectangle) if the source diagram is at zoom level > 100%.

[Phillipus]

Add the following to getViewportScaleX():

And ensure the zoom level > 100%.

System.out.println(viewport.getContents().getBounds().width);
System.out.println(getSourceRectangle().width());

You can then see that getSourceRectangle() is returning different size to the viewport.

The viewport handles different zoom levels and negative co-ordinates whereas the source figure does not.

[azoitl]

@Phillipus in 4diac IDE for both my uses cases it behaves correct. Seems we are doing something special. So this fix is doomed to day. Would you be ok if I would make the three methods in question:

• getViewportScaleX()
• getViewportScaleY()
• reconfigureSelectorBounds()

protected?

This would greatly reduce my need to duplicate code and I would not break any other application.

[Phillipus]

Seems we are doing something special.

I don't know what. Our diagrams use a ScalableFreeformRootEditPart, and a FreeformLayer and FreeformLayout for the root figure.

Would you be ok if I would make the three methods in question protected

I can't see a problem with that.

[azoitl]

Seems we are doing something special.

I don't know what. Our diagrams use a ScalableFreeformRootEditPart, and a FreeformLayer and FreeformLayout for the root figure.

We use an extended ScalableFrefromRootEditPart but with a special root figure that gives more the look and file as you have i with Draw.io. Where you have a clear border and an incremental growing canvas. I have on my todo list to bring this to GEF Classic but haven't found the time yet.

Would you be ok if I would make the three methods in question protected

I can't see a problem with that.

Then I will change my PR to that.

[Phillipus]

Seems we are doing something special.

I think I know why this is. At the point where we create the ScrollableThumbnail we set the source figure different to the viewport contents:

Viewport vp = (Viewport)editPart.getFigure();
ScrollableThumbnail thumbnail = new ScrollableThumbnail(vp);
thumbnail.setSource(editPart.getLayer(LayerConstants.PRINTABLE_LAYERS));

If I do this, your code works:

Viewport vp = (Viewport)editPart.getFigure();
ScrollableThumbnail thumbnail = new ScrollableThumbnail(vp);
thumbnail.setSource(vp.getContents());

But we prefer to set the source figure to the printable layer (and a Google search shows this is how others do it)

[azoitl]

Thx for checking and spending so much time on it. This really helped me to better understand this. In the end introducing the change only on 4diac IDE side seems the much better option.

[Phillipus]

In the end introducing the change only on 4diac IDE side seems the much better option.

I agree. At the very least we can consider that the viewport contents figure may be different than the source figure.

[azoitl]

I think with the latest update I addressed that nicely.

@Phillipus I now also know why we are not using the printable layers: If you use the whole viewport contents also selection feedback and dragging is shown in the outline. This can help some users to orient themselves. You may want to give it a try.

[Phillipus]

If you use the whole viewport contents also selection feedback and dragging is shown in the outline. This can help some users to orient themselves. You may want to give it a try.

That's why I don't use it, because I don't like that. ;-) Maybe I'll change my mind one day.

Edit: I tried it again and I see flashing black artefacts in the thumbnail when an object is dragged.

[azoitl]

If you use the whole viewport contents also selection feedback and dragging is shown in the outline. This can help some users to orient themselves. You may want to give it a try.

That's why I don't use it, because I don't like that. ;-) Maybe I'll change my mind one day.

That is the advantage of the new approach, we can have both our ways.

Edit: I tried it again and I see flashing black artefacts in the thumbnail when an object is dragged.

With the none printable layers? My fix shouldn't have any change on that, or?

Do you think we can merge it?

[Phillipus]

Edit: I tried it again and I see flashing black artefacts in the thumbnail when an object is dragged.
With the none printable layers? My fix shouldn't have any change on that, or?

Sorry, I wasn't clear - I mean I tried your suggestion of "If you use the whole viewport contents also selection feedback and dragging is shown in the outline"

Do you think we can merge it?

I checked it, and it's only code tidy, protected methods, and a new protected method in Thumbnail - so these are all non-breaking changes so looks good to me.

[azoitl]

Edit: I tried it again and I see flashing black artefacts in the thumbnail when an object is dragged.
With the none printable layers? My fix shouldn't have any change on that, or?

Sorry, I wasn't clear - I mean I tried your suggestion of "If you use the whole viewport contents also selection feedback and dragging is shown in the outline"

I expected that. But wanted to be sure that I'm accidentally breaking stuff again.

Do you think we can merge it?

I checked it, and it's only code tidy, protected methods, and a new protected method in Thumbnail - so these are all non-breaking changes so looks good to me.

Great. Then I merge and wait for the build to create the M2 release later today.