Community
Participate
Working Groups
Created attachment 270215 [details] Rendering problem on HiDPI display GEF uses images in RulerFigure to draw rotated texts in vertical rulers. This doesn't work with HiDPI images as no hires image is created. See the attached screenshot of what the result looks like on Windows with 200% scaling. I've created a patch that uses native GC rotation to draw rotated text. I figure that the code that uses rotated images dates back to the early stages of SWT, and rotated images where used as a workaround for missing rotation transform in GC. This shouldn't be a problem anymore. I'm attaching a patch that uses native rotation to fix this issue, and a screenshot that shows the end result. Please consider including this patch in the next version.
Created attachment 270216 [details] Patch that uses native rotation for drawing text in vertical rulers
Created attachment 270217 [details] The end-result of how vertical ruler looks with native rotation
Comment on attachment 270216 [details] Patch that uses native rotation for drawing text in vertical rulers Peter, thanks for contributing this. I would like to see two changes to this patch, before its ready for application: 1) Antialias requires advanced mode, so enabling the antialiasing should be guarded by a respective check to graphics.getAvanced(). 2) The common parts of the size calculations for horizontal and vertical cases should be extracted in order to not have it duplicate. Please also update the file comment header to indicate the contribution.
Created attachment 270247 [details] Patch that uses native rotation for drawing text in vertical rulers v2 Alexander, I've updated the patch as per your request. I've extracted the size calculation outside of the if block. As for the graphics.getAdvanced() guard, unfortunately this doesn't work the way you would expect. It just returns the previous value that was passed to graphics.setAdvanced(boolean). GC#getAdvanced() indeed can be used as a guard, but this is not the case for Graphics#getAdvanced(). Looking through other places where aliasing is being used, the general approach in GEF is to let the user of the library to enable anti-aliasing explicitly so GEF doesn't need to check for advanced graphics (see ConnectionLayer). Perhaps this approach could be a solution here, and anti-alias flag could be passed down into ruler via a boolean property, set by the user on GraphicalViewer, similar to RulerProvider#PROPERTY_RULER_VISIBILITY. Let me know what you think.
You seem right concerning the getAdvanced(). Yes, if following the pattern in ConnectionLayer and Shape, we would allow to set the Antialias on the RulerFigure. The problem is that it will never be accessible by any client, as the RulerComposite does not expose the ruler viewer, and a client would have to access the ruler viewer to exchange the edit part factory, in oder to change the antialias of the RulerFigure via the RulerEditPart. I have tried without enabling antialias and obtained valid results on my machine anyway. Do we need to enable it by default?
Alexander, the client doesn't have to access the ruler viewer. If you take a look at how RulerProvider properties are used, for instance RulerProvider#PROPERTY_RULER_VISIBILITY, you'll that those are set on the client's viewer that is then passed down into RulerComposite via RulerComposite#setGraphicalViewer method. By mirroring this approach I could add a RulerProvider#PROPERTY_RULER_ANTIALIAS property. I am testing this using Windows 10 in a virtual machine and the results obtained with and without anti-alias are different, as you can see in the attached screenshots. Are you testing on a Windows machine too? On Linux and Mac OS X this is not needed. I think these platforms enable anti-alias by default.
Created attachment 270281 [details] Results with and without anti-alias on Windows
(In reply to Peter Severin from comment #6) > Alexander, the client doesn't have to access the ruler viewer. If you take a > look at how RulerProvider properties are used, for instance > RulerProvider#PROPERTY_RULER_VISIBILITY, you'll that those are set on the > client's viewer that is then passed down into RulerComposite via > RulerComposite#setGraphicalViewer method. By mirroring this approach I could > add a RulerProvider#PROPERTY_RULER_ANTIALIAS property. > > I am testing this using Windows 10 in a virtual machine and the results > obtained with and without anti-alias are different, as you can see in the > attached screenshots. Are you testing on a Windows machine too? On Linux and > Mac OS X this is not needed. I think these platforms enable anti-alias by > default. I have a Mac (forgot to mention that). The graphical viewer passed to the RulerComposite is not the one containing the RulerFigure. This is placed inside a ruler viewer, which is kept internally. If not exposing a property (as you describe), we would have to access the ruler viewer (that's the problem I mentioned).
Alexander, I am not sure I understand your comment. Do you think adding a RulerProvider#PROPERTY_RULER_ANTIALIAS property is a bad idea? I didn't go into the implementation details, but RulerComposite will have to retrieve the value of this property from the client's graphical viewer and then pass it down into the ruler viewer and then into the RulerFigure. Ruler viewer won't have to be exposed directly.
(In reply to Peter Severin from comment #9) > Alexander, I am not sure I understand your comment. Do you think adding a > RulerProvider#PROPERTY_RULER_ANTIALIAS property is a bad idea? I didn't go > into the implementation details, but RulerComposite will have to retrieve > the value of this property from the client's graphical viewer and then pass > it down into the ruler viewer and then into the RulerFigure. Ruler viewer > won't have to be exposed directly. Well, passing it as a property does not seem to be really appropriate to me. I would prefer something different (but am not exactly clear what's the best option, yet).
Alexander, could we get this in without the anti-alias for now? I believe this is still better than nothing. If you are ok with this then I can create a Gerrit change for you to review.
Yes, we could do this in a first step (a patch or a GitHub pull request are sufficient, we don't use Gerrit).
Created attachment 270932 [details] Patch that uses native rotation for drawing text in vertical rulers v3 I've attached a new patch without anti-alias settings.
Alexander, do you need anything else from me for this patch to go through? Thanks!
