Deprecate ScaledGraphics

[azoitl]

All scaling needs for Draw2d can be handled by SWT natively and there are several issues in the implementation of ScaledGraphics.

FYI @HeikoKlare, @akoch-yatta

[HeikoKlare]

Thank you for informing us, @azoitl. We definitely see the benefits in SWTGraphics and would like to migrate sooner than later. We are constantly evaluating the possibility to switch from ScaledGraphics to SWTGraphics but we have so much customization implemented on top of it and so many higher priority stuff to do that we do not know yet whether that switch will be feasible for us, both technically and in terms of effort. And even if it will, we will probably not manage to perform that switch anytime soon (quite sure not until 2028-03).
I understand that this is the desired and most reasonable way for GEF to move forward, but my current expectation is that at least for us it might mean that we have to main a GEF fork starting 2028-03 to keep ScaledGraphics working for us.

[azoitl]

@HeikoKlare I undestand your concerns and that is also the reason I wanted your feedback here. I definitely don't want to drive you into a situation where you have to do a full fork of GEF Classic. I think that would harm us both.

Before discussing dates and when really to remove ScaledGraphics I wanted to point out alternatives for you with little effort. How I see ScaledGraphics it is a very isolated components with no dependencies. It is just created in certain methods. If you have an enhanced version of ScaledGraphics you already needed to hook into GEF Classic and provide your class instead of ScaledGraphics. I definitely want to keep such hooks and if we do stuff that makes this harder for you I would like to know.

If me analysis is correct then you could just copy ScaledGraphics into your code base and continue from there without the need to do a full fork of GEF Classic.

Aside from that I'm not sure what would be a good switching time. Maybe 2028-03 was to optimistic and early. I'm open to discuss this.

Also I one thing that I did with this PR is to switch the default from ScaledGraphics to SWTGraphics. My Idea behind that is to get early feedback where SWTGraphics is not yet a good replacement for ScaledGraphics. But again this can be discussed.

[ptziegler]

To provide some additional context: The deprecation has been proposed quite some time ago with Bug 442442 - Deprecate ScaledGraphics, make native scaling the default.

The purpose of this class is to enable scaling on operating systems that don't support advanced mode. This should no longer be a concern:

According to SWT FAQ, Advanced Graphics is supported by SWT in the following cases:

• SWT for Windows uses GDI+ for advanced graphics, which is installed on Windows XP and newer. Windows 2000 users can download and install a Microsoft package containing GDI+.
• SWT for GTK+ and Motif use Cairo for advanced graphics, which is installed on systems with GTK+ 2.8 or newer (for example RHEL 5).
• SWT for OS X uses Quartz for advanced graphics, which is installed on all supported OS X versions.

However, I think it has become clear that this class can also be used (some might say misused) in a different context. I'm not sure if also marking it as "for removal" is the best approach, especially because we already know that our own editor examples rely on some of the side effects of the ScaledGraphics class.

Deprecating sounds fine to be but I think we should first make sure that we don't depend on it ourselves, before we start telling other projects to move away from it.

[ptziegler]

I remember there being issues with the Logic editor when scaling with a plain SWTGraphics object. I'd have to double-check if this is still the case, but I would prefer to not change the default behavior when this already causes issues for our examples.

[azoitl]

I remember too. That is also my hesitation in my comment above. However I thought we could change it now early so that we find such problems and can address them and if we can not address them we can still change it back in M3 or RC1. Similar to the native scaling flag we introduced in the last cycle.

[ptziegler]

I think it makes sense to focus on one thing at a time. So for now, I would propose to only deprecated the ScaledGraphics class and cleanup the PrinterGraphics class at a later stage.

[azoitl]

I think it makes sense to focus on one thing at a time. So for now, I would propose to only deprecated the ScaledGraphics class and cleanup the PrinterGraphics class at a later stage.

Good point. Will remove this change and later look into how @Phillipus is doing this in Archi. Because this was the driver of the original API change around PrinterGraphics.

[Phillipus]

Good point. Will remove this change and later look into how @Phillipus is doing this in Archi. Because this was the driver of the original API change around PrinterGraphics.

Mine is a bit of hack, see https://github.com/archimatetool/archi/blob/a033f986cf68ff3f5659ab7d9c5c3a11631ff3b2/org.eclipse.draw2d/src/org/eclipse/draw2d/PrintOperation.java#L31

(I need to align my hacked fork of GEF/Draw2d with the latest version sometime next year)

[Phillipus]

I found that ScaledGraphics handles scaling of fonts better in some cases, but worse in others. But things might be different now with later versions of SWT.

[azoitl]

@Phillipus thx for the feedback. Removed the deprecation from PrinterGraphics.

[HeikoKlare]

Thank you again for letting us know and asking for feedback in advance!

If you have an enhanced version of ScaledGraphics you already needed to hook into GEF Classic and provide your class instead of ScaledGraphics.

I did not check yet but I am quite sure that it is true that we could just integrate the parts we need from ScaledGraphics into our extensions (or just copy the whole class over to our codebase).
The only thing why I would be a bit concerned about is that all upstream testing/validation is not done for ScaledGraphics anymore when it gets removed from the codebase. So potential incompatibilities may show up later than they do now. And it may even be the case that at some points where ScaledGraphics is somehow special, they might not work anymore in the future as what ScaledGraphics is/does right now will be considered incorrect then.
We can be optimistic that this will not happen, but I am not that confident based on how often I have seen implementations we did in SWT for Windows being contract-conforming in the first place but then turned out to be incompatible with the Linux and MacOS implementations. Transferred to the scenario here, one might do changes to Graphics/SWTGraphics and rely on them at consumers that are intended to be implementation agnostic (as SWTGraphics is the only relevant/available Graphics implementation then) but which are incompatible with the current state of ScaledGraphics, delegating the burden to adapt to all GEF consumers that still require and thus copied ScaledGraphics. That's also why it's usually good to have multiple implementations of very central interfaces, so that you better notice unintended specifics in one of the implementations.
With all that said, please don't get me wrong: I know about the problems with ScaledGraphics and agree that getting rid of it is the best one can do. I just want to open/add further viewpoints on potential effects of that.

I will share the information about the intent to possibly remove ScaledGraphics so that we can base proper decisions on that. We know about the benefits of SWTGraphics, but we still need to find out if and with how much effort we could migrate everything to it. Even with copying ScaledGraphics being a temporary option for some time after a potential removal from the framework, I don't think it's a reasonable and even feasible long-term solution due to the points mentioned above.
But for now, we still have quite some time left 😃

[HeikoKlare]

@ptziegler just found by accident that at least the LogicEditor does not look correct with monitor scaling (e.g., 150%) anymore.
This is how it should look:

And this is how it actually looks now:

When I revert this change to initialize the root edit part for the LogicEditor with ScaledGraphics again, the issue is fixed.

[ptziegler]

I remember mentioning the problem here as well: #824 (comment)

There are two problems, actually:

The first one is the improper placement of the figures, which is because the SWTGraphics class is using an Transform object for scaling. You can get the same issue at 100% monitor zoom by changing the editor zoom to e.g. 400%, so it's a general issue.

I'm not able to reproduce this on Linux, so I'm not sure yet whether this is an inconsistency on the Draw2D or the SWT side.

The second one is a side-effect introduced by enabling advanced scaling. Normally, the feedback figure should be transparent, but this is never explicitly set (in fact. getAlpha() returns 255). When advanced mode is enabled this causes the feedback figures to always be drawn opaque.

[HeikoKlare]

I currently try to adapt our product to SWTGraphics being used by default. This means, I have to adapt all places that now default to SWTGraphics instead of ScaledGraphics to explicitly use ScaledGraphics instead (until we have eventually migrated to SWTGraphics). I run into some complications as not all consumers of the classes with changed defaults can easily be reconfigured. This includes:

• GraphicalViewerImpl/ScrollingGraphicalViewer create a ScalableRootEditPart in createDefaultRoot(). Every instantiation of those classes would have to be overwritten with a subclass that replaces this method with one that creates a ScalabelRootEditPart with ScaledGraphics. Every existing subclass needs to overwrite that method additionally
• ScalableLightweightSystem is instantiated at several places that are not easy to replace, such as RulerViewer or PopupHelper, which in turn are instantiated at other places that would have to be overwritten as well.
• SimpleAutoscaledRootEditPart creates a ScalableLayeredPane in createFigure(). The class is internal and currently only consumed by the PaletteViewer for which using SWTGraphics should hopefully be fine. Still I wanted to mentioned as we have a consumer in out product (comparable to the PaletteViewer), which probably needs to configure the pane to use ScaledGraphics.

The first one produces some effort and for the second I am not even sure how one could use the ScalableLightweightSystem() and its consumer with ScaledGraphics if desired.

@ptziegler the Changelog file mentiones the classes for which the default configuration has been changed but does not propose a strategy how to switch back to ScaledGraphics for them, in particular including all the non-configurable consumers of them. Is there any strategy how to properly switch back or do consumers potentially have to overwrite all consumers classes (such as GraphicalViewerImpl and the like) to preserve the existing behavior?

[azoitl]

@HeikoKlare thx for your analysis. It looks that my initial proposal of switching the default to SWTGraphics was to radical. What I could over to reduce the effort for now is to change the default back to use ScaledGraphics and still allow clients to opt out of ScaledGraphics. But keep ScaledGraphics deprecated.

[ptziegler]

I remember bringing up the same concern. My understanding was that we try out switchting to SWTGraphics by default and reconsider if there are complications.

#901 (comment)

I remember too. That is also my hesitation in my comment above. However I thought we could change it now early so that we find such problems and can address them and if we can not address them we can still change it back in M3 or RC1. Similar to the native scaling flag we introduced in the last cycle.

To be honest, I haven't put much thought into the migration step, primarily because I don't really have dog in this fight.

[azoitl]

I remember bringing up the same concern. My understanding was that we try out switchting to SWTGraphics by default and reconsider if there are complications.

#901 (comment)

And with @HeikoKlare's feedback we achieved my goal.

I remember too. That is also my hesitation in my comment above. However I thought we could change it now early so that we find such problems and can address them and if we can not address them we can still change it back in M3 or RC1. Similar to the native scaling flag we introduced in the last cycle.

To be honest, I haven't put much thought into the migration step, primarily because I don't really have dog in this fight.

As long as I can easily switch the major parts to SWTGraphics I'm fine too. The reason for my approach was to find out the problems of SWTGraphics. This we definitely did. And to hopefully reduce the amount of code we have to maintain. Therefore I will not introduce new api so that all use-case raised by @HeikoKlare could be addressed. We better switch back for now. I'll do a PR early next week. However I would like to switch then the logic editor to SWTGraphics as it is really a good litmus test.

[HeikoKlare]

I hope I didn't produce any confusion with the @ mention, as I just accidentally had in mind that Patrick created this PR while actually Alois did it. The information/question was more a general one and not supposed to point to someone specific 🙂

I understand that I did not miss any specific, existing idea how to switch back to ScaledGraphics everywhere needed but just found some places that may not be properly configurable right now.
I can also try to add the necessary API for our use cases and see if I can make all required places configurable. Then I might create a PR that you could extend with further places that I might have missed. Maybe I can even manage to have a test run for our product based on those changes soon and then report back.

[HeikoKlare]

It seems to be quite impossible to make every single place configurable. Just as an example, the PaletteViewer creates a SimpleAutoscaledRootEditPart, which now by default uses SWTGraphics. If we wanted to allow to configure that, you would need to add the configuration to the PaletteViewer, which is instantiated by PaletteViewerProvider that then also requries the configuration, which is instantiated by a GraphicsEditorWithFlyoutPalette (or any of its subclasses) that then also also require the configuration and so on.
Now it should usually be fine to use SWTGraphics for a PaletteViewer. For us this is currently just a problem because we have one single remaining patch for GEF which modifies the SWTGraphics in a way that makes it incompatible with this use case, which is why we have to switch back to ScaledGraphics here as well until we have properly extracted/replaces that patch.

A more relevant case might be the GraphicalViewer that creates as default root edit part a ScalableRootEditPart with SWTGraphics. Such a viewer is instantiated in the GraphicalEditor, such as a GraphicalEditorWithPalette or any other concrete implementation, so that you would have to overwrite the createGraphicalViewer() with an implementation that creates the viewer with ScaledGraphics in each actual GraphicalEditor that you want to use. That's not impossible but also quite cumbersome.

So I wonder if it might not be helpful to have a global switch for the default Graphics implementation to use by default to support the transition to SWTGraphics. For us, something like that would definitely be helpful (or even necessary) as we would otherwise need to patch or partly revert the switch to SWTGraphics by default again. And it would avoid that you have to decide between one default or the other and a bunch of potentially new temporary API to make every place configurable, but you could stick to SWTGraphics being the default with a global switch back to ScaledGraphics. More precisely, instead of reverting your change of defaults, just do not replace true with false for useScaledGraühics parameters but replace it with some configurable value (e.g., customized via system property).
@azoitl what do you think? I will give this a try for our product as fine-grained configurability does currently not seem to be a suitable approach for us anyway.
Edit: I have created a PR demonstrating the idea, which seems to work fine with our product (which is somehow expected as it allows to simply revert the change of defaults):

• Make default Graphics implementation configurable #1002