<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#c9">Comment # 9</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:cgarcia@igalia.com" title="Carlos Garcia Campos <cgarcia@igalia.com>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=282789&action=diff" name="attach_282789" title="Patch">attachment 282789</a> <a href="attachment.cgi?id=282789&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=282789&action=review">https://bugs.webkit.org/attachment.cgi?id=282789&action=review</a>
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:-606
> -#if USE(COORDINATED_GRAPHICS_THREADED)
> - return;
> -#elif USE(TEXTURE_MAPPER_GL) && !USE(COORDINATED_GRAPHICS)
> - if (client())
> - return;
> -#endif
> -</span >
So, paint was not supposed to be called before when using AC, right? Can this still be called when using AC but not rendering in accelerated 2d canvas context?
<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));</span >
The caller null checks the return value, when is this supposed to return null? So, we are now returning an uninitialized surface?
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:697
> - return nullptr;
> + return adoptRef(cairo_gl_surface_create(device, CAIRO_CONTENT_COLOR_ALPHA, 0, 0));</span >
Ditto.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:702
> cairo_surface_t* surface = cairo_gl_surface_create_for_texture(device, CAIRO_CONTENT_COLOR_ALPHA, textureID, size.width(), size.height());</span >
Instead of using raw pointers and then create the RefPtr when returning it, we could change this to use RefPtr, so that if an early return is added after this we won't leak the surface.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:703
> + cairo_surface_mark_dirty(surface);</span >
I'm surprised we need to mark the surface as dirty right after creating it, and before render into it.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:706
> + cairo_surface_t* rotatedSurface = cairo_gl_surface_create(device, CAIRO_CONTENT_COLOR_ALPHA, rotatedSize.width(), rotatedSize.height());</span >
Use RefPtr here.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:707
> + cairo_t* cr = cairo_create(rotatedSurface);</span >
And here.
<span class="quote">> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:717
> + case OriginRightTop: {
> + cairo_translate(cr, rotatedSize.width() * 0.5, rotatedSize.height() * 0.5);
> + cairo_rotate(cr, M_PI / 2.0);
> + cairo_translate(cr, -rotatedSize.height() * 0.5, -rotatedSize.width() * 0.5);
> + break;
> + }</span >
Why do you need {} here and not in the other cases?</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>