JetBrains / JetBrainsRuntime

Runtime environment based on OpenJDK for running IntelliJ Platform-based products on Windows, macOS, and Linux
GNU General Public License v2.0
1.3k stars 193 forks source link

wayland: implement fractional scaling #384

Closed mahkoh closed 4 months ago

mahkoh commented 4 months ago

This PR implements fractional scaling.

On KDE and Jay, this makes IntelliJ render crisply at every scale that I have tested, including 125%, 133%, 150%, and 175%.

Under GNOME, IntelliJ will not render crisply at some scales including 175%. This seems to be a bug in GNOME since other applications are also affected by this.

Unfortunately there appear to be a number of "off by one" errors in the drawing logic of awt or swing or the IntelliJ toolkit that causes the interface to be off at certain scales. This is most easily seen at scale 150% where selecting or hovering over certain components causes them to move by one pixel.

Curiously, this seems to happen whenever there is some kind of overlay. For example, in the following screenshot you can see the IntelliJ quick-action tooltip that causes everything below it to shift down by one pixel.

20240521_07h42m42s_grim

One suspicion is that this is caused by client-side compositing that reads back the buffer and, after compositing, does not write back the pixels to the correct position. I'll have to investigate this further.


Additionally, this PR fixes some issues that I had to debug to make sure that they were not caused by my changes.

mahkoh commented 4 months ago

The issue I mentioned above with the UI jumping around should now be fixed. It was indeed so that data was copied between different buffers of the same scale but the x and y coordinates were rounded multiple times in different ways which caused the copies to end up in incorrect places.

There is still an issue that some fill operations fill one pixel too much or too little. But it's only a minor inconvenience.

mahkoh commented 4 months ago

wayland: don't commit partially incomplete buffers

Besides removing some amount of flickering, I believe this also fixes a case where IntelliJ would sometimes segfault when copying the damaged regions to the show buffer.

mkartashev commented 4 months ago

Under GNOME, IntelliJ will not render crisply at some scales including 175%. This seems to be a bug in GNOME since other applications are also affected by this.

Indeed; see https://mail.openjdk.org/pipermail/wakefield-dev/2024-May/000167.html That,however, already precludes me from working further on this patch up until Gnome/Mutter has resolved those issues as the majority of IDEA users are running Gnome/Mutter. There's no sense in considerably complicating the code without at least some benefit for the great majority of users.

In fact, I implemented the support for fractional scaling (JBR-6969) a few days back and was profoundly disappointed with the overall quality of the result, see this note. With a hindsight, I shouldn't have expected much: after all, when you need a straight vertical line of one pixel and desktop's scale is 175%, you must draw that line 1.75 pixels wide. This will never be as crisp as 1 or 2 or 3 pixels wide.

At this point I believe over-scaling might be a better solution (see JBR-7193): paint at 2x or 4x and let the compositor down-scale to the proper size, assuming it will employ hardware acceleration and decent image shrinking algorithms. At this moment, when Wayland uses fractional scale JBR will slightly over-scale the resulting image (rounding upwards to the nearest integer), so some over-scaling is already present and it already produces better results than with no over-scaling at all. For example, the left hand side of this image is produced by over-scaling to 200% and the right hand side at desktop scale of 150%: image

There are several further issues with the patch; to name a few:

  1. When I run SwingSet2 (./build/linux-x86_64-server-release/images/jdk/demo/jfc/SwingSet2/SwingSet2.jar), it crashes with
    Exception in thread "AWT-EventQueue-0" sun.java2d.InvalidPipeException: Invalid Image variant
    at java.desktop/sun.awt.image.SurfaceManager.getManager(SurfaceManager.java:82)
    at java.desktop/sun.java2d.SunGraphics2D.drawImage(SunGraphics2D.java:3480)
    at java.desktop/sun.java2d.SunGraphics2D.drawImage(SunGraphics2D.java:3449)
    at java.desktop/javax.swing.plaf.metal.MetalUtils$GradientPainter.paintImage(MetalUtils.java:309)
    at java.desktop/sun.swing.CachedPainter.paint0(CachedPainter.java:217)
    at java.desktop/sun.swing.CachedPainter.paint(CachedPainter.java:114)
    at java.desktop/javax.swing.plaf.metal.MetalUtils$GradientPainter.paint(MetalUtils.java:270)
    at java.desktop/javax.swing.plaf.metal.MetalUtils.drawGradient(MetalUtils.java:223)
    at java.desktop/javax.swing.plaf.metal.MetalMenuBarUI.update(MetalMenuBarUI.java:109)
    ...
  2. Stylepad (build/linux-x86_64-server-release/images/jdk/demo/jfc/Stylepad/Stylepad.jar) UI bounces during an interactive resize as if it changes its size for a fraction of a second and then it goes back again.
  3. There are quite a few changes in the shared code; so far we have been able to limit this to a tiny change in the repaint manager; changing shared code for the sake of one Toolkit is a big red flag and needs to be studied very carefully.
  4. I'm not sure what are all the ramifications of using one graphics config per window as opposed to per monitor. This also needs to be studied carefully.
  5. There are changes in the handling of the memory buffers, which are probably unrelated to fractional scaling. The current version (sans the patch) crashes under KDE; I will look into that separately.
mahkoh commented 4 months ago

That,however, already precludes me from working further on this patch up until Gnome/Mutter has resolved those issues as the majority of IDEA users are running Gnome/Mutter. There's no sense in considerably complicating the code without at least some benefit for the great majority of users.

I'm sure the vast majority of IDEA users don't use linux at all and yet IDEA has complicated its code to run on linux.

But in any case, the screenshots at the bottom of this message show that this PR also significantly improves results on GNOME.

In fact, I implemented the support for fractional scaling (JBR-6969) a few days back and was profoundly disappointed with the overall quality of the result, see this note.

I did test that code but the implementation seems to have been incorrect. The results were sometimes blurry even on non-GNOME compositors.

With a hindsight, I shouldn't have expected much: after all, when you need a straight vertical line of one pixel and desktop's scale is 175%, you must draw that line 1.75 pixels wide. This will never be as crisp as 1 or 2 or 3 pixels wide.

AWT will always round such things to integers. I.e. a 1 java-pixel line will be rounded up to 2 buffer-pixels when drawing at 175% scale. I can assure you that everything is 100% crisp on non-GNOME compositors.

This screenshot shows RustRover rendered at 175% scale:

20240523_12h19m16s_grim

Here is part of the image zoomed in:

zoomed-in

As you can see, everything is aligned to the pixel grid.

You will never get crisp results on any compositors if you force the compositor to downscale.

For example, the left hand side of this image is produced by over-scaling to 200% and the right hand side at desktop scale of 150%:

Did you create this screenshot with your branch or with the code in this PR? At 150% scale, GNOME is able to render fractional scaling 100% crisply on my system. Compare the screenshots I've taken (on GNOME) with this patch

gnome-150-frac

and without this patch

gnome-150-int

I believe the results with this patch are much better. So this patch would be advantageous even for GNOME users.

There are several further issues with the patch; to name a few:

I'll look into these but when I tested SwingSet2 it worked fine.

mahkoh commented 4 months ago

When I run SwingSet2 (./build/linux-x86_64-server-release/images/jdk/demo/jfc/SwingSet2/SwingSet2.jar), it crashes with

I've been able to reproduce this and it should be fixed now. This was caused by one of my last changes that I only tested with IntelliJ.

Stylepad (build/linux-x86_64-server-release/images/jdk/demo/jfc/Stylepad/Stylepad.jar) UI bounces during an interactive resize as if it changes its size for a fraction of a second and then it goes back again.

I've not been able to reproduce this. Stylepad behaves the same with or without this PR. Maybe this was fixed by my fix for the previous issue.

There are quite a few changes in the shared code; so far we have been able to limit this to a tiny change in the repaint manager; changing shared code for the sake of one Toolkit is a big red flag and needs to be studied very carefully.

There are two changes outside of pure-wayland code:

The changes to the repaint manager fix an issue with your previous changes where you would call wl_surface_commit from within the RepaintManager.paint function. This was incorrect. The RepaintManager has not finished drawing until after RepaintManager.endPaint has been called. This was the cause of significant flickering during resize (with or without fractional scaling). I don't see a way to do this without changing the call sites of the repaint manager as I've done in this PR.

The changes to SunGraphics2D are necessary to get accurate buffer copies when the scale is fractional. These need to be reviewed carefully.

There are changes in the handling of the memory buffers, which are probably unrelated to fractional scaling. The current version (sans the patch) crashes under KDE; I will look into that separately.

I believe this crash was caused by the following:

  1. the code acquires the draw lock
  2. the code checks if the show buffer must be resized
  3. the code resizes the show buffer
  4. the code releases the draw lock
  5. the code acquires the draw lock
  6. the code copies the draw buffer to the show buffer

If the draw buffer was resized between 4 and 5, this could segfault. My change makes it so that the draw lock is held for the entire time. (There is slightly more to it but this is the gist of it.)

mkartashev commented 4 months ago

If the draw buffer was resized between 4 and 5, this could segfault. My change makes it so that the draw lock is held for the entire time. (There is slightly more to it but this is the gist of it.)

Yes, makes sense. This needs to be addressed separately and urgently. I will see to that - JBR-7198

mahkoh commented 4 months ago

All of the commits except the last one are not essential to this PR. Each of them fixes a more or less serious bug but none are required for fractional scaling to work. If it would help moving this forward, I could extract them to separate pull requests. The fractional scaling commit does not touch any non-wayland code.

(Some of them might depend on your viewporter code so opening them in the wakefield repo would have to wait.)

Edit: Actually I guess the java2d: implement drawImage as a copy if src & dst have the same scale commit is rather important because otherwise IntelliJ will flicker with fractional scaling, but it's only a single function that should be easy to review.

mkartashev commented 4 months ago

I could extract them to separate pull requests

Let's start from the beginning: please, file bugs (one per problem), describing issues that you spotted, how to reproduce them and/or what specific problems do you see in the code, etc.

mahkoh commented 4 months ago

https://youtrack.jetbrains.com/issue/JBR-7202/wayland-memory-leak-when-resizing-windows for wayland: use weak surface manager references in property listeners

https://youtrack.jetbrains.com/issue/JBR-7203/wayland-window-is-corrupted-during-resize for wayland: defer commit until double-buffered state has been flushed

You've already created https://youtrack.jetbrains.com/issue/JBR-7198 which should cover wayland: don't commit partially incomplete buffers

java2d: implement drawImage as a copy if src & dst have the same scale cannot be observed without fractional scaling I think. I'll see if I can reproduce this on X11.

mahkoh commented 4 months ago

I was able to reproduce the last issue by using -Dsun.java2d.uiScale=1.5: https://youtrack.jetbrains.com/issue/JBR-7204/wayland-window-contents-jump-around-at-1.5-scale

mahkoh commented 4 months ago

Annoyingly github doesn't seem to support re-targeting a PR when the base-branch is merged. I've opened #388 as a replacement.