<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Painting a video into a canvas doesn't work when accelerated compositing is enabled"
href="https://bugs.webkit.org/show_bug.cgi?id=159405#c13">Comment # 13</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [GTK] Painting a video into a canvas doesn't work when accelerated compositing is enabled"
href="https://bugs.webkit.org/show_bug.cgi?id=159405">bug 159405</a>
from <span class="vcard"><a class="email" href="mailto:magomez@igalia.com" title="Miguel Gomez <magomez@igalia.com>"> <span class="fn">Miguel Gomez</span></a>
</span></b>
<pre><span class="quote">> > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:692
> > > > - return nullptr;
> > > > + return adoptRef(cairo_gl_surface_create(device, CAIRO_CONTENT_COLOR_ALPHA, 0, 0));
> > >
> > > The caller null checks the return value, when is this supposed to return
> > > null? So, we are now returning an uninitialized surface?
> >
> > Yes, the caller checks for null value. But hen trying to create a pattern
> > from the video content, if null is returned CanvasRenderingContext2D will
> > fall back to use paintCurrentFrameInContext, which may crash. I found this
> > problem when trying to get a frame from a video that is not playing yet
> > using gstreamer-gl. In that situation neither the frame nor the caps are
> > available. That's why I return an empty surface instead, so the rendering
> > context doesn't try to paint again.
>
> So, maybe that should be handled in paint? I'm not sure I understand how
> this is supposed to work. Because then paint() is not supposed to be called
> not either, if the canvas always uses nativeImageForCurrentTime() in case of
> accelerated 2d canvas, no?</span >
Ok, I'll modify the patch to return null again, and open a new bug to handle the situation where the caps are not negotiated in another bug.
<span class="quote">> > > > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:703
> > > > + cairo_surface_mark_dirty(surface);
> > >
> > > I'm surprised we need to mark the surface as dirty right after creating it,
> > > and before render into it.
> >
> > That's because we are creating the surface for a texture that already has
> > content, but cairo doesn't know. So When using cairo to paint it, as there
> > were no cairo painting operations, it assumes that there's nothing to paint.
> > Marking it as dirty tells cairo that there's already content to use.
>
> This sounds to me like a cairo bug, as I understand it, mark dirty is to
> tell cairo that you have modified the data of an existing surface. The image
> backend, marks the surface as dirty when cairo_image_surface_create_for_data
> is called and the given data is not NULL. I guess the gl backend should do
> something similar when a surface is created with a texture. If this is the
> case, we should file a bug report in cairo, add a comment here saying this
> is a workaround for a cairo bug and adding bug url in the comment, and if we
> also fix cairo that would be awesome :-)</span >
I've been retesting this and seems that the mark_dirty is not necessary. I added it when using other approaches to fix the issue, but seems that with this approach it's working properly without it.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>