GNOME Bugzilla – Bug 771553
Shrinking window generates a black patch when gl is used
Last modified: 2016-09-28 08:20:29 UTC
Created attachment 335732 [details] Screenshot showing the problem I am not sure why this does not affect the GLArea widget. In the WebKitGTK+ test browser, when we use gdk_cairo_draw_from_gl to paint the final rendering to GTK+, we get this weird behaviour. My investigation points to the tracking of the opaque region. The opaque region is properly set on the GdkWindowWayland, but never synced. When using the glReadPixels codepath in WebKitGTK+, the opaque region is synced for every frame. The reason seems to be pending_commit is never true (except for a few early frames), so the call to sync_opaque_region is never reached: static void on_frame_clock_after_paint (GdkFrameClock *clock, GdkWindow *window) { GdkWindowImplWayland *impl = GDK_WINDOW_IMPL_WAYLAND (window->impl); struct wl_callback *callback; if (!impl->pending_commit) return; … gdk_wayland_window_sync_margin (window); gdk_wayland_window_sync_opaque_region (window); gdk_wayland_window_sync_input_region (window); The reason why pending_commit is never true is it is only updated when current_paint.use_gl is not true: static void gdk_window_impl_wayland_end_paint (GdkWindow *window) { GdkWindowImplWayland *impl = GDK_WINDOW_IMPL_WAYLAND (window->impl); cairo_rectangle_int_t rect; int i, n; if (!window->current_paint.use_gl && !cairo_region_is_empty (window->current_paint.region)) { … impl->pending_commit = TRUE; } }
Created attachment 335733 [details] [review] Patch that works around or fixes the problem This patch fixes the issue for me. I am not sure it is the best solution, though, I'm uploading it in hopes someone more acquainted to the code can propose or help me figure out the best solution.
I was sent here after filing https://bugs.webkit.org/show_bug.cgi?id=162317 It turns out that this became a visible issue on GNOME 3.22.0 once webkitgtk 2.14.0 was used. This basically now renders GNOME 3.22.0 on wayland with any kind of webkit2 apps (webkit / epiphany) pretty broken
(In reply to Dominique Leuenberger from comment #2) > I was sent here after filing https://bugs.webkit.org/show_bug.cgi?id=162317 > > It turns out that this became a visible issue on GNOME 3.22.0 once webkitgtk > 2.14.0 was used. I'm not sure if this bug report encompasses the entirety of the issue we discussed or not. I'd like to see Bjørn's screenshot of the seriously broken shell overview make its way onto Bugzilla (still got it?), so that the right developers can decide if we need a separate bug report for that or not and where if so. > This basically now renders GNOME 3.22.0 on wayland with any > kind of webkit2 apps (webkit / epiphany) pretty broken Yes, it's a shame that this was caused by a very last-minute WebKit change. We should consider reverting the WebKit patch at least for now due to the severity of this issue. But I won't ask about that until a GTK+ hacker looks at this first, since maybe Gustavo's patch is all we need.
Created attachment 336105 [details] "boken" overview (hidpi) Sure I still got it :-)
Review of attachment 335733 [details] [review]: I'm sad to say I don't think this is the correct solution. The reason for this is that we may end up with one incorrect frame (the one committed by eglSwapBuffers()), that'll then be fixed up by setting the rest of the state (window geometry, opaque region, etc) in the another commit (the one in on_frame_clock_after_paint()). What exactly is it in on_frame_clock_after_paint() that makes the issue go away? Is it the gdk_wayland_window_sync_...() calls? I think those could probably be safely moved to gdk_window_impl_wayland_end_paint().
Created attachment 336250 [details] [review] wayland: always sync state after a frame is painted It is indeed the sync_opaque_region call that fixes it. Moving the sync calls worked for me. =)
(In reply to Bjørn Lie from comment #4) > Created attachment 336105 [details] > "boken" overview (hidpi) > > Sure I still got it :-) How do you reproduce that? Do you just start yelp or do you resize it somehow to get that result?
(In reply to Gustavo Noronha (kov) from comment #7) > (In reply to Bjørn Lie from comment #4) > > Created attachment 336105 [details] > > "boken" overview (hidpi) > > > > Sure I still got it :-) > > How do you reproduce that? Do you just start yelp or do you resize it > somehow to get that result? Have webkitgtk 2.14.0 installed. Start yelp, epiphany or evolution (any webkitgtk based app). Resize the app, get black areas (when making smaller) or get areas not "sensitive" when making bigger.
(In reply to Gustavo Noronha (kov) from comment #6) > Created attachment 336250 [details] [review] [review] > wayland: always sync state after a frame is painted > > It is indeed the sync_opaque_region call that fixes it. Moving the sync > calls worked for me. =) This works for me too +1 for this patch!
(In reply to Bjørn Lie from comment #9) > (In reply to Gustavo Noronha (kov) from comment #6) > > Created attachment 336250 [details] [review] [review] [review] > > wayland: always sync state after a frame is painted > > > > It is indeed the sync_opaque_region call that fixes it. Moving the sync > > calls worked for me. =) > > This works for me too +1 for this patch! I can add that when built combined with the patch from bug 771915 -> totem fullscreen is fine too
(In reply to Bjørn Lie from comment #10) > I can add that when built combined with the patch from bug 771915 -> totem > fullscreen is fine too Nope, not here, and I fail to see how attachment 336250 [details] [review] could help with bug 771915
*** Bug 771944 has been marked as a duplicate of this bug. ***
Review of attachment 336250 [details] [review]: I think this is safe to do. I think the reason why we don't do wl_surface_commit() in this function is because it may be called multiple times per frame per window, which 'after-paint' will not be, but since these sync functions already throttle themself (by having the dirty state), and the fact that we'll always see the eglSwapBuffers() and 'after-paint' handler being called *after* syncing the state now, lets go with this.
Comment on attachment 336250 [details] [review] wayland: always sync state after a frame is painted Attachment 336250 [details] pushed as 7292b03 - wayland: always sync state after a frame is painted