<html>
<head>
<base href="https://bugzilla.gnome.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [Wayland] Crash under gdk_wayland_window_attach_image()"
href="https://bugzilla.gnome.org/show_bug.cgi?id=773274#c20">Comment # 20</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [Wayland] Crash under gdk_wayland_window_attach_image()"
href="https://bugzilla.gnome.org/show_bug.cgi?id=773274">bug 773274</a>
from <span class="vcard"><a href="page.cgi?id=describeuser.html&login=ofourdan%40redhat.com" title="Olivier Fourdan <ofourdan@redhat.com>"> <span class="fn">Olivier Fourdan</span></a>
</span></b>
<pre>(In reply to Ray Strode [halfline] from <a href="show_bug.cgi?id=773274#c18">comment #18</a>)
<span class="quote">> ohhhhh okay, where's the source to the example you're running? I wonder why
> its doing gdk_flush.</span >
The reproducer is the one attached in <a class="bz_bug_link
bz_status_NEW "
title="NEW - [Wayland] crash at gdk_flush() called from "draw" signal handler during resize"
href="show_bug.cgi?id=773307">bug 773307</a>.
<span class="quote">> If it continues to draw after the gdk_flush it's going to write to a cairo
> surface that won't ultimately get committed but I guess that's okay.
>
> As an aside, I think maybe we should have:
>
> g_clear_pointer (&impl->staged_updates_region, cairo_region_destroy);
>
> in drop_cairo_surfaces since the staged updates just got dropped.</span >
Possibly but I'm not sure it's necessary for this particular bug, is it? (but I
have no problem amending the patch if needed. or even add a separate patch for
that)
(In reply to Ray Strode [halfline] from <a href="show_bug.cgi?id=773274#c19">comment #19</a>)
<span class="quote">> Review of <span class=""><a href="attachment.cgi?id=338945&action=diff" name="attach_338945" title="[PATCH] wayland: check valid pending cairo surface">attachment 338945</a> <a href="attachment.cgi?id=338945&action=edit" title="[PATCH] wayland: check valid pending cairo surface">[details]</a></span> <a href='review?bug=773274&attachment=338945'>[review]</a> [review]:
>
> ::: gdk/wayland/gdkwindow-wayland.c
> @@ +906,3 @@
>
> + if (impl->staging_cairo_surface &&
> + _gdk_wayland_is_shm_surface (impl->staging_cairo_surface) &&
>
> i don't think the is_shm_surface check is necessary ?</span >
In theory, I agree, but it's how it's done elsewhere in
gdk_wayland_window_ensure_cairo_surface():
<a href="https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n824">https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n824</a>
Besides, there is an assert() in gdk_wayland_window_attach_image() which would
fail precisely if not a shm_surface():
<a href="https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n733">https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c?h=gtk-3-22#n733</a></pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>