[Webkit-unassigned] [Bug 159405] [GTK] Painting a video into a canvas doesn't work when accelerated compositing is enabled

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=159405

--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 282789
  --> https://bugs.webkit.org/attachment.cgi?id=282789
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282789&action=review

> 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
> -

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?

> 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?

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:697
> -        return nullptr;
> +        return adoptRef(cairo_gl_surface_create(device, CAIRO_CONTENT_COLOR_ALPHA, 0, 0));

Ditto.

> 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());

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.

> 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.

> 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());

Use RefPtr here.

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:707
> +    cairo_t* cr = cairo_create(rotatedSurface);

And here.

> 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;
> +    }

Why do you need {} here and not in the other cases?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160706/975af07d/attachment-0001.html>