After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 762120 - [Wayland] Black content when using testfilechooser auto completer
[Wayland] Black content when using testfilechooser auto completer
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2016-02-16 05:16 UTC by Jonas Ådahl
Modified: 2016-02-16 22:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: Handle after-paint invocations when nothing was painted (3.08 KB, patch)
2016-02-16 05:16 UTC, Jonas Ådahl
committed Details | Review
wayland: Only attach the buffer if there was damage (1.44 KB, patch)
2016-02-16 05:17 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2016-02-16 05:16:52 UTC
When triggering the autocomplete window of testfilechooser (e.g. testfilechooser -a save) black content will displayed occasionally when entering text in the text entry (text that would trigger autocompletion).

I'm attaching two patches, one that fixes the issue, and another one that is just a related simplification and optimization.
Comment 1 Jonas Ådahl 2016-02-16 05:16:57 UTC
Created attachment 321340 [details] [review]
wayland: Handle after-paint invocations when nothing was painted

If a after-paint was scheduled but nothing was painted, for example when
the it was scheduled by a subsurface wanting to update its position,
we'd still try to read back from the backfill cairo surface and update
the committed cairo surface reference even though no buffer was
attached.

Fix this by adding a new state, 'pending_buffer_attached', which is only
true if a buffer was attached during frame. Only when this is true will
the backfill be read back and the committed cairo surface reference be
updated.
Comment 2 Jonas Ådahl 2016-02-16 05:17:03 UTC
Created attachment 321341 [details] [review]
wayland: Only attach the buffer if there was damage

There is no point in attaching and then committing the same buffer if
there was no damage. This will also make us do less unnecessary backfill
read backs, for the cases where we paint with an empty paint region.
Comment 3 Ray Strode [halfline] 2016-02-16 21:50:09 UTC
Review of attachment 321341 [details] [review]:

sure, makes sense to me.
Comment 4 Ray Strode [halfline] 2016-02-16 22:03:57 UTC
Review of attachment 321340 [details] [review]:

so this looks fine to me, but it does nicely illustrate an interesting point: a commit operation could be one or more different things.  I wonder if a cleaner approach than adding another boolean, would be to change the pending_commit variable to take a bitfield of commit flags instead?  Then you could set pending_commit |= PENDING_DAMAGE; pending_commit |= PENDING_BUFFER_ATTACHMENT; pending_commit |= SUBSURFACE_SET_POSITION etc etc.

and inside on_frame_clock_after_paint you could do
 
if (pending_commit & PENDING_DAMAGE)
    read_back_cairo_surface (...)
...

 if (pending_commit & PENDING_BUFFER_ATTACHMENT) 
   impl->committed_cairo_surface = ...

just an idea, what you've got seems fine.
Comment 5 Ray Strode [halfline] 2016-02-16 22:07:52 UTC
actually i'm going to push these out now since Matthias is making a release tonight.  if you want to do the bitfield thing later, we can always just track it on another bug.
Comment 6 Ray Strode [halfline] 2016-02-16 22:08:36 UTC
Attachment 321340 [details] pushed as 64c9ec1 - wayland: Handle after-paint invocations when nothing was painted
Attachment 321341 [details] pushed as 38b1f15 - wayland: Only attach the buffer if there was damage