[ozone] Commit the surface tree after SetShape() called [chromium/src : main]
Thomas Lukaszewicz (Gerrit)
Thomas Lukaszewicz uploaded patch set #2 to this change.
[ozone] Commit the surface tree after SetShape() called
Currently SetShape() sets double buffered state on the shell surface
corresponding to the toplevel window server side [1]. This state is
applied to the root surface only after a commit is received from the
client for the surface.
In some circumstances this can lead to the shape not taking effect
immediately (see crbug.com/1158733#c29). This CL ensures that clients
propagate a commit after SetShape() has been called for the top level
window, making sure that the change occurs server-side shortly after.
[1] https://source.chromium.org/chromium/chromium/src/+/main:components/exo/shell_surface_base.h;l=448;drc=eef4762779d05708d5dfc7d5fe4ea16288069a35
Bug: 1158733, 1417600
Change-Id: I3298358d7566125e61d27619eb72e0da130958f2
---
M ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
1 file changed, 4 insertions(+), 0 deletions(-)
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Patch set 2:Commit-Queue +1
1 comment:
Patchset:
Oshima ptal, quick fix to ensure shape changes take effect server side shortly after they are requested by the client.
I don't think this happens practically with existing apps using SetShape() since commits are sent regularly - but there are circumstances where the effect is not seen due to a commit not having been received on the server.
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Attention is currently required from: Mitsuru Oshima.
Thomas Lukaszewicz would like Mitsuru Oshima to review this change.
[ozone] Commit the surface tree after SetShape() called
Currently SetShape() sets double buffered state on the shell surface
corresponding to the toplevel window server side [1]. This state is
applied to the root surface only after a commit is received from the
client for the surface.
In some circumstances this can lead to the shape not taking effect
immediately (see crbug.com/1158733#c29). This CL ensures that clients
propagate a commit after SetShape() has been called for the top level
window, making sure that the change occurs server-side shortly after.
[1] https://source.chromium.org/chromium/chromium/src/+/main:components/exo/shell_surface_base.h;l=448;drc=eef4762779d05708d5dfc7d5fe4ea16288069a35
Bug: 1158733, 1417600
Change-Id: I3298358d7566125e61d27619eb72e0da130958f2
---
M ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
1 file changed, 4 insertions(+), 0 deletions(-)
diff --git a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
index 8981131..7b59c29 100644
--- a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
@@ -321,6 +321,10 @@
const gfx::Transform& transform) {
if (shell_toplevel_) {
shell_toplevel_->SetShape(std::move(native_shape));
+ // The surface shape is double-buffered state maintained by the shell
+ // surface server-side and applied to the root surface. We must also commit
+ // the surface tree to ensure state is applied correctly.
+ root_surface()->Commit(false);
}
}
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Attention is currently required from: Mitsuru Oshima.
Thomas Lukaszewicz uploaded patch set #3 to this change.
[ozone] Commit the surface tree after SetShape() called
Currently SetShape() sets double buffered state on the shell surface
corresponding to the toplevel window server side [1]. This state is
applied to the hosting widget only after a commit is received from
the client for the root surface.
In some circumstances this can lead to the shape not taking effect
immediately (see crbug.com/1158733#c29). This CL ensures that clients
propagate a commit after SetShape() has been called for the top level
window, making sure that the change occurs server-side shortly after.
[1] https://source.chromium.org/chromium/chromium/src/+/main:components/exo/shell_surface_base.h;l=448;drc=eef4762779d05708d5dfc7d5fe4ea16288069a35
Bug: 1158733, 1417600
Change-Id: I3298358d7566125e61d27619eb72e0da130958f2
---
M ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
1 file changed, 4 insertions(+), 0 deletions(-)
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Mitsuru Oshima.
Mitsuru Oshima (Gerrit)
Attention is currently required from: Thomas Lukaszewicz.
1 comment:
File ui/ozone/platform/wayland/host/wayland_toplevel_window.cc:
Patch Set #3, Line 327:
root_surface()->Commit(false);I think we probably should have a way to tell if we're in the middle of configure, or pending commit?
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Mitsuru Oshima (Gerrit)
Attention is currently required from: Thomas Lukaszewicz.
Patch set 3:Code-Review +1
1 comment:
Patchset:
oops sent too early. This CL lgtm. I was wondering if we should have a way to commit at the end of task.
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Attention is currently required from: Thomas Lukaszewicz.
Thomas Lukaszewicz uploaded patch set #4 to this change.
[ozone] Commit the surface tree after SetShape() called
Currently SetShape() sets double buffered state on the shell surface
corresponding to the toplevel window server side [1]. This state is
applied to the hosting widget only after a commit is received from
the client for the root surface.
In some circumstances this can lead to the shape not taking effect
immediately (see crbug.com/1158733#c29). This CL ensures that clients
propagate a commit after SetShape() has been called for the top level
window, making sure that the change occurs server-side shortly after.
[1] https://source.chromium.org/chromium/chromium/src/+/main:components/exo/shell_surface_base.h;l=448;drc=eef4762779d05708d5dfc7d5fe4ea16288069a35
Bug: 1158733, 1417600, 1481057
Change-Id: I3298358d7566125e61d27619eb72e0da130958f2
---
M ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
1 file changed, 4 insertions(+), 0 deletions(-)
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Attention is currently required from: Thomas Lukaszewicz.
Patch set 4:Commit-Queue +2
findit-for-me@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Thomas Lukaszewicz.
This change meets the code coverage requirements.
Patch set 4:Code-Coverage +1
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
[ozone] Commit the surface tree after SetShape() called
Currently SetShape() sets double buffered state on the shell surface
corresponding to the toplevel window server side [1]. This state is
applied to the hosting widget only after a commit is received from
the client for the root surface.
In some circumstances this can lead to the shape not taking effect
immediately (see crbug.com/1158733#c29). This CL ensures that clients
propagate a commit after SetShape() has been called for the top level
window, making sure that the change occurs server-side shortly after.
[1] https://source.chromium.org/chromium/chromium/src/+/main:components/exo/shell_surface_base.h;l=448;drc=eef4762779d05708d5dfc7d5fe4ea16288069a35
Bug: 1158733, 1417600, 1481057
Change-Id: I3298358d7566125e61d27619eb72e0da130958f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4846858
Reviewed-by: Mitsuru Oshima <osh...@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tl...@chromium.org>
Code-Coverage: findit...@appspot.gserviceaccount.com <findit...@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1194886}
---
M ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
1 file changed, 4 insertions(+), 0 deletions(-)
diff --git a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
index 67dd8b9..414648d 100644
--- a/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
+++ b/ui/ozone/platform/wayland/host/wayland_toplevel_window.cc
@@ -326,6 +326,10 @@
const gfx::Transform& transform) {
if (shell_toplevel_) {
shell_toplevel_->SetShape(std::move(native_shape));
+ // The surface shape is double-buffered state maintained by the shell
+ // surface server-side and applied to the root surface. We must also commit
+ // the surface tree to ensure state is applied correctly.
+ root_surface()->Commit(false);
}
}
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Lukaszewicz (Gerrit)
Patch Set #3, Line 327:
root_surface()->Commit(false);
I think we probably should have a way to tell if we're in the middle of configure, or pending commit […]
Yeah we should, I created crbug.com/1481057 to track the work needed for this and we can address this in a follow up (didn't publish comment before +2).
To view, visit change 4846858. To unsubscribe, or for help writing mail filters, visit settings.