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.21k stars 188 forks source link

JBR-7158 Wayland: scale with wp_viewport instead of buffer scale #382

Closed mkartashev closed 1 month ago

mkartashev commented 2 months ago

JBR-7158 Wayland: scale with wp_viewport instead of buffer scale

Scaling with wp_viewport is necessary in case we wanted to adopt fractional scaling at some point in the future.

mahkoh commented 1 month ago

The rubber banding that you described in my fractional scaling PR happens because you sometimes set the viewport destination size and commit the surface without attaching a new buffer. The following patch fixes this:

diff --git a/src/java.desktop/unix/native/common/java2d/wl/WLBuffers.c b/src/java.desktop/unix/native/common/java2d/wl/WLBuffers.c
index 145ad55ccd4..00a82af5ca2 100644
--- a/src/java.desktop/unix/native/common/java2d/wl/WLBuffers.c
+++ b/src/java.desktop/unix/native/common/java2d/wl/WLBuffers.c
@@ -49,10 +49,10 @@ ScheduleFrameCallback(WLSurfaceBufferManager * manager);
 static void
 CancelFrameCallback(WLSurfaceBufferManager * manager);

-static void
+static bool
 CopyDrawBufferToShowBuffer(WLSurfaceBufferManager * manager);

-static void
+static bool
 SendShowBufferToWayland(WLSurfaceBufferManager * manager);

 static inline void
@@ -244,7 +244,6 @@ struct WLDrawBuffer {
  */
 struct WLSurfaceBufferManager {
     struct wl_surface * wlSurface;         // only accessed under showLock
-    bool                sendBufferASAP;    // only accessed under showLock
     int                 bgPixel;           // the pixel value to be used to clear new buffers
     int                 format;            // one of enum wl_shm_format

@@ -655,21 +654,20 @@ ShowBufferIsAvailable(WLSurfaceBufferManager * manager)
     return canSendMoreBuffers;
 }

+static bool
+IsFrameCallbackScheduled(WLSurfaceBufferManager * manager);
+
 static void
-TrySendShowBufferToWayland(WLSurfaceBufferManager * manager, bool sendNow)
+TrySendShowBufferToWayland(WLSurfaceBufferManager * manager)
 {
-    WLBufferTrace(manager, "TrySendShowBufferToWayland(%s)", sendNow ? "now" : "later");
-
-    sendNow = sendNow && ShowBufferIsAvailable(manager);
-    if (sendNow) {
-        CopyDrawBufferToShowBuffer(manager);
-        SendShowBufferToWayland(manager);
-    } else {
-        ScheduleFrameCallback(manager);
-    }
+    WLBufferTrace(manager, "TrySendShowBufferToWayland");

+    if (IsFrameCallbackScheduled(manager)) return;
+    if (!ShowBufferIsAvailable(manager)) return;
+    if (!CopyDrawBufferToShowBuffer(manager)) return;
+    if (!SendShowBufferToWayland(manager)) return;
+    ScheduleFrameCallback(manager);
     WLBufferTrace(manager, "wl_surface_commit");
-    // Need to commit either the damage done to the surface or the re-scheduled callback.
     wl_surface_commit(manager->wlSurface);
 }

@@ -690,7 +688,7 @@ wl_frame_callback_done(void * data,
     if (manager->wlSurface) {
         const bool hasSomethingToSend = (manager->bufferForDraw.damageList != NULL);
         if (hasSomethingToSend) {
-            TrySendShowBufferToWayland(manager, true);
+            TrySendShowBufferToWayland(manager);
         }
         // In absence of damage, wait for another WLSBM_SurfaceCommit() instead of waiting
         // for another frame; the latter may never bring anything different in case of
@@ -737,7 +735,7 @@ CancelFrameCallback(WLSurfaceBufferManager * manager)
  * Attaches the current show buffer to the Wayland surface, notifying Wayland
  * of all the damaged areas in that buffer.
  */
-static void
+static bool
 SendShowBufferToWayland(WLSurfaceBufferManager * manager)
 {
     ASSERT_SHOW_LOCK_IS_HELD(manager);
@@ -748,7 +746,7 @@ SendShowBufferToWayland(WLSurfaceBufferManager * manager)
     WLSurfaceBuffer * buffer = manager->bufferForShow.wlSurfaceBuffer;
     if (buffer == NULL) {
         // There should've been an OOME thrown already
-        return;
+        return false;
     }

     // We'll choose a free buffer or create a new one for the next frame
@@ -759,10 +757,6 @@ SendShowBufferToWayland(WLSurfaceBufferManager * manager)
     wl_surface_attach(manager->wlSurface, buffer->wlBuffer, 0, 0);
     //NB: do not specify scale for the buffer; we scale with wp_viewporter

-    // Better wait for the frame event so as not to overwhelm Wayland with
-    // frequent surface updates that it cannot deliver to the screen anyway.
-    manager->sendBufferASAP = false;
-
     DamageList_SendAll(manager->bufferForShow.damageList, manager->wlSurface);
     DamageList_FreeAll(manager->bufferForShow.damageList);
     manager->bufferForShow.damageList = NULL;
@@ -778,6 +772,8 @@ SendShowBufferToWayland(WLSurfaceBufferManager * manager)
     if (manager->frameSentCallback != NULL) {
         manager->frameSentCallback(manager->surfaceDataWeakRef);
     }
+
+    return true;
 }

 static void
@@ -822,7 +818,7 @@ CopyDamagedArea(WLSurfaceBufferManager * manager, jint x, jint y, jint width, ji
  *
  * Updates the damaged areas in all the existing free and in-use buffers.
  */
-static void
+static bool
 CopyDrawBufferToShowBuffer(WLSurfaceBufferManager * manager)
 {
     ASSERT_SHOW_LOCK_IS_HELD(manager);
@@ -830,7 +826,7 @@ CopyDrawBufferToShowBuffer(WLSurfaceBufferManager * manager)

     if (manager->bufferForShow.wlSurfaceBuffer == NULL || manager->bufferForDraw.data == NULL) {
         MUTEX_UNLOCK(manager->drawLock);
-        return;
+        return false;
     }

     assert(manager->wlSurface != NULL);
@@ -872,6 +868,8 @@ CopyDrawBufferToShowBuffer(WLSurfaceBufferManager * manager)
     WLBufferTrace(manager, "CopyDrawBufferToShowBuffer: copied %d area(s) in %lldns", count, endTime - startTime);

     MUTEX_UNLOCK(manager->drawLock);
+
+    return true;
 }

 static void
@@ -984,7 +982,6 @@ WLSBM_SurfaceAssign(WLSurfaceBufferManager * manager, struct wl_surface* wl_surf
     MUTEX_LOCK(manager->showLock);
     if (manager->wlSurface == NULL || wl_surface == NULL) {
         manager->wlSurface = wl_surface;
-        manager->sendBufferASAP = true; // ...so that this new surface association is made known to Wayland
         // The "frame" callback depends on the surface; when changing the surface,
         // cancel any associated pending callbacks:
         CancelFrameCallback(manager);
@@ -1082,9 +1079,8 @@ WLSBM_SurfaceCommit(WLSurfaceBufferManager * manager)
                   manager->wlSurface,
                   frameCallbackScheduled ? "wait for frame" : "now");

-    if (manager->wlSurface && !frameCallbackScheduled) {
-        // Don't always send the frame immediately so as not to overwhelm Wayland
-        TrySendShowBufferToWayland(manager, manager->sendBufferASAP);
+    if (manager->wlSurface) {
+        TrySendShowBufferToWayland(manager);
     }
     MUTEX_UNLOCK(manager->showLock);
 }
@@ -1125,14 +1121,6 @@ WLSBM_SizeChangeTo(WLSurfaceBufferManager * manager, jint width, jint height)
         manager->bufferForDraw.height = height;
         manager->bufferForDraw.resizePending = true;

-        // Send the buffer at the nearest commit or else Mutter may not remember
-        // the latest size of the window.
-        manager->sendBufferASAP = true;
-
-        // Need to wait for WLSBM_SurfaceCommit() with the new content for
-        // the buffer size, so there's no need for the frame event until then.
-        CancelFrameCallback(manager);
-
         WLBufferTrace(manager, "WLSBM_SizeChangeTo %dx%d", width, height);
     }
mkartashev commented 1 month ago

The rubber banding that you described in my fractional scaling PR happens because you sometimes set the viewport destination size and commit the surface without attaching a new buffer

Certainly possible. What do you mean by "rubber banding" specifically?

mahkoh commented 1 month ago

What do you mean by "rubber banding" specifically?

I meant this part:

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.

When you increase the size of the window, you sometimes commit the new viewport size before you have attached a new buffer, thereby making the window look stretched. When you then attach the new, correctly sized buffer, the window snaps back to being correctly proportioned.

mkartashev commented 1 month ago

When you increase the size of the window, you sometimes commit the new viewport size before you have attached a new buffer, thereby making the window look stretched. When you then attach the new, correctly sized buffer, the window snaps back to being correctly proportioned.

I see, thanks. So the buffer resizing and window resizing should go hand in hand then. The code needs to be restructured to make this happen naturally.

mahkoh commented 1 month ago

The code needs to be restructured to make this happen naturally.

Indeed, the current split between the buffer object and the surface object makes this quite complicated.