1215082 - [Wayland] - Workaround for CAIRO_SUBSURFACE types

Status: RESOLVED WONTFIX

(Core :: Graphics, enhancement)

Description

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

+++ This bug was initially created as a clone of Bug #635134 +++

Wayland supply us cairo surface with CAIRO_SUBSURFACE type. It's erroneously reported as image surface type but cairo_image_* routines fail on them. So let's report them as unknow/default cairo surface.

Comment 1

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Attachment: patch

A patch. Jeff, I wonder who is a correct reviewer for this - please reassign if I'm wrong.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8674188 [details] [diff] [review]
patch

Review of attachment 8674188 [details] [diff] [review]:
-----------------------------------------------------------------

Is this when built with system cairo? How are these surfaces ending up in thebes/moz2d?

Comment 3

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Yes, that's with system cairo. Those subsurfaces are provided by GTK in expose-event (draw event) for actual drawing to GdkWindow. We also get the subsurface when query GdkWindow for cairo context on non-X11 backends (Wayland, Broadway).

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8674188 [details] [diff] [review]
patch

Review of attachment 8674188 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/2d/DrawTargetCairo.cpp
@@ +667,5 @@
> +    if(format != CAIRO_FORMAT_INVALID) {
> +        return CairoFormatToGfxFormat(format);
> +    } else {
> +        // Try to workaround a cairo bug when subsurfaces are reported
> +        // as image surfaces but cairo_image_ functions fails.

It seems like cairo is intentionally reporting subsurfaces as image_surfaces.

@@ +668,5 @@
> +        return CairoFormatToGfxFormat(format);
> +    } else {
> +        // Try to workaround a cairo bug when subsurfaces are reported
> +        // as image surfaces but cairo_image_ functions fails.
> +        return CairoContentToGfxFormat(cairo_surface_get_content(surface));

Out of curiosity, do you have a call stack for when this happens. It's not obvious to me when we would be getting the format for a surface from gtk.

::: gfx/thebes/gfxASurface.cpp
@@ +173,5 @@
> +            result = new gfxImageSurface(csurf);
> +        } else {
> +            // Try to workaround a cairo bug when subsurfaces are reported
> +            // as image surfaces but cairo_image_ functions fails.
> +            result = new gfxUnknownSurface(csurf, aSize);

I'm also interested in knowing the call stack for this too.

Comment 5

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

> ::: gfx/thebes/gfxASurface.cpp
> @@ +173,5 @@
> > +            result = new gfxImageSurface(csurf);
> > +        } else {
> > +            // Try to workaround a cairo bug when subsurfaces are reported
> > +            // as image surfaces but cairo_image_ functions fails.
> > +            result = new gfxUnknownSurface(csurf, aSize);
> 
> I'm also interested in knowing the call stack for this too.

That comes from a patch for Bug 1215104 - it's directly called from nsWindow::GetThebesSurface():

6428	            cairo_t *cr = gdk_cairo_create(mGdkWindow);
6429	            cairo_surface_t *surf = cairo_get_target(cr);
6430	            if (cairo_surface_status(surf) != CAIRO_STATUS_SUCCESS) {
6431	                NS_NOTREACHED("Missing cairo target?");
6432	                return nullptr;
6433	            }
6434	            mThebesSurface = gfxASurface::Wrap(surf, size);

Comment 6

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> Out of curiosity, do you have a call stack for when this happens. It's not
> obvious to me when we would be getting the format for a surface from gtk.

#0  0x00007fffe338d074 in mozilla::gfx::GfxFormatForCairoSurface(_cairo_surface*) (surface=0x7fffc24ec820)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/gfx/2d/DrawTargetCairo.cpp:672
#1  0x00007fffe35bd73c in gfxASurface::GetSurfaceFormat() const (this=0x7fffc1e58610)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/gfx/thebes/gfxASurface.cpp:723
#2  0x00007fffe35db9d8 in gfxPlatform::CreateDrawTargetForSurface(gfxASurface*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&) (this=0x7fffd0b2e900, aSurface=0x7fffc1e58610, aSize=...)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/gfx/thebes/gfxPlatform.cpp:741
#3  0x00007fffe526b009 in nsWindow::StartRemoteDrawing() (this=0x7fffca37ff00)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/widget/gtk/nsWindow.cpp:6347
#4  0x00007fffe526065e in nsWindow::OnExposeEvent(_cairo*) (this=0x7fffca37ff00, cr=0x7fffc219e000)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/widget/gtk/nsWindow.cpp:2189
#5  0x00007fffe5268c8d in draw_window_of_widget(_GtkWidget*, _GdkWindow*, _cairo*) (widget=0x7fffca6e2f70, aWindow=0x7fffca6a7030, cr=0x7fffc219e000) at /home/komat/tmp676-trunk-gtk3/gecko-dev/widget/gtk/nsWindow.cpp:5335
#6  0x00007fffe5268d73 in expose_event_cb(GtkWidget*, cairo_t*) (widget=0x7fffca6e2f70, cr=0x7fffc219e000)
    at /home/komat/tmp676-trunk-gtk3/gecko-dev/widget/gtk/nsWindow.cpp:5358

Comment 7

[Jeff Muizelaar [:jrmuizel]]

(In reply to Martin Stránský from comment #6)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> > Out of curiosity, do you have a call stack for when this happens. It's not
> > obvious to me when we would be getting the format for a surface from gtk.
> 
> #0  0x00007fffe338d074 in
> mozilla::gfx::GfxFormatForCairoSurface(_cairo_surface*)
> (surface=0x7fffc24ec820)

This call stack looks out of date. Is the problem still happening?

Comment 8

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Yes, the problem is still here as the SUBSURFACE is not correctly recognized by gfxASurface::Wrap() and handled as CAIRO_SURFACE_TYPE_IMAGE in GfxFormatForCairoSurface().

Comment 9

[Martin Stránský [:stransky] (ni? me) (PTO, back on Feb 17)]

Not needed anymore.