Bug 190697 – Add new image type for CSS painting API

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Justin Michaud]

Add new image type for CSS painting API, and hook it up to draw something.

[Justin Michaud]

Created attachment 352681 [details]
Patch

[Justin Michaud]

Created attachment 352710 [details]
Patch

[Dean Jackson]

Comment on attachment 352710 [details]
Patch

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

> Source/WebCore/platform/graphics/CustomPaintImage.cpp:76
> +    auto canvas = OffscreenCanvas::create(*scriptExecutionContext, destSize.width(), destSize.height());
> +    ExceptionOr<OffscreenRenderingContext> contextOrException = canvas->getContext(*execState, OffscreenCanvas::RenderingContextType::Webgl, { });
> +
> +    if (contextOrException.hasException())
> +        return ImageDrawResult::DidNothing;
> +    auto context = contextOrException.releaseReturnValue();
> +
> +    context->clearColor(0, 0, 0, 1.0);
> +    context->clear(GL_COLOR_BUFFER_BIT);
> +
> +    auto result = m_paintCallback->handleEvent(*context);
> +    if (result.type() != CallbackResultType::Success)
> +        return ImageDrawResult::DidNothing;
> +
> +    auto bitmap = canvas->transferToImageBitmap();
> +    if (!bitmap)
> +        return ImageDrawResult::DidNothing;
> +
> +    destContext.drawImage(*bitmap->buffer()->copyImage(), FloatPoint());

This will work fine but will be too slow in practice, since transferToImageBitmap is a GPU readback. However, it's a good place to start.

Ultimately I don't expect we'll want to support WebGL initially - only 2d contexts. It's a bit scary to think that any element on a page could require a WebGL canvas/context.

[Justin Michaud]

(In reply to Dean Jackson from comment #3)
> Comment on attachment 352710 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=352710&action=review
> 
> > Source/WebCore/platform/graphics/CustomPaintImage.cpp:76
> > +    auto canvas = OffscreenCanvas::create(*scriptExecutionContext, destSize.width(), destSize.height());
> > +    ExceptionOr<OffscreenRenderingContext> contextOrException = canvas->getContext(*execState, OffscreenCanvas::RenderingContextType::Webgl, { });
> > +
> > +    if (contextOrException.hasException())
> > +        return ImageDrawResult::DidNothing;
> > +    auto context = contextOrException.releaseReturnValue();
> > +
> > +    context->clearColor(0, 0, 0, 1.0);
> > +    context->clear(GL_COLOR_BUFFER_BIT);
> > +
> > +    auto result = m_paintCallback->handleEvent(*context);
> > +    if (result.type() != CallbackResultType::Success)
> > +        return ImageDrawResult::DidNothing;
> > +
> > +    auto bitmap = canvas->transferToImageBitmap();
> > +    if (!bitmap)
> > +        return ImageDrawResult::DidNothing;
> > +
> > +    destContext.drawImage(*bitmap->buffer()->copyImage(), FloatPoint());
> 
> This will work fine but will be too slow in practice, since
> transferToImageBitmap is a GPU readback. However, it's a good place to start.
> 
> Ultimately I don't expect we'll want to support WebGL initially - only 2d
> contexts. It's a bit scary to think that any element on a page could require
> a WebGL canvas/context.

I did that because it was the only offscreen canvas I could get easily. The spec actually only supports 2d.

[WebKit Commit Bot]

Comment on attachment 352710 [details]
Patch

Clearing flags on attachment: 352710

Committed r237276: <https://trac.webkit.org/changeset/237276>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45390166>

[Simon Fraser (smfr)]

Comment on attachment 352710 [details]
Patch

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

In general this approach doesn't seem right. We should not be firing the callback synchronously at paint time. We should be firing the call back at requestAnimationFrame time; the context drawn into should be backed by a display list, and in this doCustomPaint code we should just be playing back that display list.

> Source/WebCore/css/CSSPaintCallback.h:41
> +    virtual CallbackResult<void> handleEvent(WebGLRenderingContext&) = 0;

Why is this using WebGLRenderingContext? The custom paint API is al 2d.

> Source/WebCore/platform/graphics/CustomPaintImage.cpp:59
> +    ExceptionOr<OffscreenRenderingContext> contextOrException = canvas->getContext(*execState, OffscreenCanvas::RenderingContextType::Webgl, { });

Shouldn't be WebGL.