Implement a monitor-aware lightweight system to handle HiDPI scaling

[ptziegler]

This lightweight-system disables the native HiDPI scaling that is normally done by SWT and instead scales the figures via a ScalableLayeredPane.

The main purpose of this class is to reduce the noise that is introduced by rounding, errors when dealing with fractional scaling.

To test this functionality, simply pass the MonitorAwareLightweighSystem as an additional argument when constructing the FigureCanvas. See the AutoScaleExample for reference

[ptziegler]

@akoch-yatta FYI, this is the solution I eventually came up with. The scaling is done by a scalable figure that is added before the other figures.

[ptziegler]

For example, this is the flow editor with the palette scaled at 300% (and the editor at 100%):

Some open topics:

• The name of the classes are obviously inspired by the SWT classes. Though there might be better names for them.
• Images are blurry. This is because the image data is taken based on the SWT zoom and then scaled to match the Draw2D zoom. Though this only happens if draw2d.autoScale is used.
  Similar problems also appear when calculating e.g. the text sizes..

[ptziegler]

Some additional notes:

• This generally works until the user replaces the viewport. Which is what happens in the ScalableRootEditPart, so things aren't as simply for GEF.

• It would be cleaner to swap viewport and scalable pane. But then the figures are painted at 100% zoom, but only on Windows, only when scrolling and even if the SWTGraphics instance is painting the correctly scaled shapes.

[akoch-yatta]

Never thought of adding the scaling logic into the Viewport. I will try that one out. Looks more concise than my approach. Only problem is probably, that the monitor zoom is not incorporated into the diagram zoom, which affects fonts and images.

One question from my side: Why do you put that logic into a subclass of Viewport and not into Viewport itself? Is it because you can overwrite parts of it, and you could break the whole logic? I know it is a Windows only "feature" and hidden behind an opt-in (for now). But in my opinion this behavior should be transparently used without the need to use a specific class, because it is hiding an existing limitation of Windows, that the developer of a GEF plug-in cannot control, e.g. with swt.autoScale=quarter and a zoom of 125% you will have artifacts when scrolling through a diagram otherwise.

[akoch-yatta]

I played a bit around with it. The current solution is not suitable for the palette, correct? You need to apply the same pattern to palette as well (drawing and scaling on a 100% GC). Otherwise you will have artifacts when scrolling in a palette as well. But the Viewport doesn't work here or does it?

[ptziegler]

@akoch-yatta

Why do you put that logic into a subclass of Viewport and not into Viewport itself? Is it because you can overwrite parts of it, and you could break the whole logic? I know it is a Windows only "feature" and hidden behind an opt-in (for now). But in my opinion this behavior should be transparently used without the need to use a specific class, because it is hiding an existing limitation of Windows, that the developer of a GEF plug-in cannot control, e.g. with swt.autoScale=quarter and a zoom of 125% you will have artifacts when scrolling through a diagram otherwise.

Touching the Viewport is very scary and at least for now, this functionality should be strictly opt-in. Who knows in what creative ways this class is used by consumers and I can't predict all the ways this might break when suddenly additional figures are injected.

When the MonitorAwareLightweightSystem is used, the return type of Viewport.getContents() is also changed from a customer-set IFigure to the ScalableLayeredPane created by this LWS. This is necessary for the scrollbar-calculation and a significant behavioral change.

This is still a draft and perhaps I can find a better way to hide this pane. In which case, this can be made configurable via a system property or something. Having this functionality be "invisible" and without adaptations to the user-code would of course be the ideal solution.

The current solution is not suitable for the palette, correct? You need to apply the same pattern to palette as well (drawing and scaling on a 100% GC). Otherwise you will have artifacts when scrolling in a palette as well. But the Viewport doesn't work here or does it?

Every FigureCanvas has a Viewport, so it shouldn't matter whether it's used with the palette, the editor or any other custom component. The problem with my previous implementation was that everything breaks, the moment you set your own Viewport, which is something that happens in the ScalableRootEditPart. Which is also why some parts where scaled, while others were not:

gef-classic/org.eclipse.gef/src/org/eclipse/gef/editparts/ScalableRootEditPart.java

Lines 173 to 182 in 50e27e3

	@Override
	protected IFigure createFigure() {
	Viewport viewport = createViewport();
	innerLayers = new LayeredPane();
	createLayers(innerLayers);
	viewport.setContents(innerLayers);
	return viewport;
	}

I assume this is also what you meant with this, correct?

Only problem is probably, that the monitor zoom is not incorporated into the diagram zoom, which affects fonts and images.

I have adapted this PR to also consider this and inject the ScalableLayeredPane on-the fly, without having to create a dedicated MonitorAwareViewport.

This is the editor now, scaled at 200%.

Just as a side-note: This is a problem with Draw2D, so it's dangerous to focus on GEF because the same issue also exists in e.g. Zest. Any solution should be universal and sadly limits the avenues from which this problem can be approached...

[akoch-yatta]

@ptziegler thanks for the update.

@HeikoKlare and me tried it today, but immediately ran into a brickwall when trying it with an editor with a FreeformViewport e.g. the LogicEditor. We spent a bit of time with it to try to make it work and it was usable with the small changes you'll find in https://github.com/vi-eclipse/gef-classic/tree/monitor-aware-lws, but did not work in other zooms except 100%, because it runs into an endless validation loop when the freeform bounds were set and reset in scaled and unscaled values. I assume there is a proper solution possible for that current limitation?

[ptziegler]

@akoch-yatta Can you explain how you reproduced the issue with the LogicEditor? I opened it at 150% zoom without any issues, so I assume there is some additional configuration necessary.

Or do you have a small snippet that I could try out?

[HeikoKlare]

@ptziegler probably the relevant change we made is that we changed GraphicsViewerImpl#createLightweightSystem() to return a MonitorAwareLightweightSystem. When using this PR with GEF out of the box, the MonitorAwareLightweightSystem will not be used for the LogicEditor. Maybe we need to instantiate the MonitorAwareLightweightSystem somewhere else?

Note that with the change above some issues pop up. In particular, FreeformViewport assumes that its content is a FreeformFigure, but now became a ScalableLayeredPane. This is the patch I applied (Andreas had something similiar): vi-eclipse@0ab8db3
But as indicated by the code comment, this only works for monitor zoom 100, as with a different monitor zoom the logic for setting the bounds runs into an invalidation loop of figures in the diagram.

[ptziegler]

@HeikoKlare Thanks, that was it. After a rebase, the changes to the GraphicsViewerImpl got reverted, so the new class wasn't used. There is probably a way around that as well, but I get the impression that this approach also becomes more and more messy...

[HeikoKlare]

Seems like it's not so easy to find a good and generic solution. We will probably integrate a preliminary solution (based on what Andreas has proposed) into our product soon to allow the usage of monitor-specific scaling without introducing the artifact line issues in GEF diagrams. We will keep sharing our findings with that approach and look forward to syncing on and finding a generic solution to GEF/Draw2d that we can finally adopt.

[ptziegler]

Superseded by #770