exo: Pointer::SetCursor() appears to not correctly snapshot the cursor surface |
|||||
Issue descriptionI observed that a sequence similar to the following leads to an incorrect mouse cursor being used afterwards: // client alpha fades out cursor, last step... zcr_blending_v1_set_alpha(cursor_surface_blending, 0.1); wl_surface_commit(cursor_surface); // client decides cursor is fully hidden, sets null surface... wl_pointer_set_cursor(pointer, 0, cursor_surface, 0, 0) // client decides cursor should be visible zcr_blending_v1_set_alpha(cursor_surface_blending, 1.0); wl_surface_commit(cursor_surface); wl_pointer_set_cursor(pointer, 0, cursor_surface, 0, 0) It happens that as above if the next to last commited surface has a low but not quite zero alpha value, that the end result after all the calls was that Chrome would use a very translucent mouse cursor, rather than a fully opaque cursor. After looking at the code in pointer.cc, it looks like it tries to take a snapshot of the mouse cursor by having it rendered to a buffer, and then setting that buffer as the bitmap to use via other platform specific calls. As a workaround, I could add an extra wl_surface_commit(cursor_surface) after making the wl_pointer_set_cursor call(), and that would be enough to correctly snapshot the fully opaque cursor bitmap. But really wl_pointer_set_cursor() should be fixed so that it correctly snapshots the cursor set as the new surface.
,
Aug 23 2017
Thanks for detailed description. Let's fix set_cursor code in chrome if wayland protocol doesn't require a commit after making a surface a cursor.
,
Aug 24 2017
After suggesting a quick fix (Calling SurfaceTreeHost::OnSurfaceCommit() in Pointer::SetCursor()), reveman@ suggested that a better fix was to fix how Surface::Commit() handles things when there is no delegate. We want the surface to handle the commit action when not given a role that causes it to be displayed in any way. This would mean setting the current state to the pending state (including handling buffer changes and releases) so that if it is output afterwards, it will be shown the the correct current state. Additionally we may need to change how Pointer captures the snapshot to ensure that it does get the updated surface. Currently the snapshotting process does not ensure Surface::AppendContentsToFrame() is called so that the pointer root window output has the correct output frame.
,
Aug 24 2017
Lowering the priority, since I will be using the workaround instead for now.
,
Aug 27
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 27
We may have fixed it. Or we may have not. Either way we have since switched to a pointer shape type as the primary way of specifying the cursor. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by lpique@chromium.org
, Aug 23 2017