Bug 237260 - [GPU Process] Canvas compositing buffer should be created through its GraphicsContext
Summary: [GPU Process] Canvas compositing buffer should be created through its Graphic...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
: 236929 (view as bug list)
Depends on:
Blocks: 236508
  Show dependency treegraph
 
Reported: 2022-02-27 15:52 PST by Said Abou-Hallawa
Modified: 2022-03-04 11:41 PST (History)
11 users (show)

See Also:


Attachments
Patch (3.37 KB, patch)
2022-02-27 15:55 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.96 KB, patch)
2022-03-02 12:14 PST, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 2022-02-27 15:52:36 PST
This will guarantee the newly created ImageBuffer will inherit all the canvas drawing settings: colorSpace, renderingMode and renderingMethod. So if the backend of the underlying ImageBuffer of the canvas is remote the compositing ImageBuffer will also be remote. This will transfer the whole compositing operation to GPUProcess.

This bug causes the layout test fast/canvas/canvas-composite-canvas.html to crash with the following call stack:

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.WebKit              	0x0000000105ed3ef8 WTFCrashWithInfo(int, char const*, char const*, int) + 20 (Assertions.h:741)
1   com.apple.WebKit              	0x00000001065412f8 WebKit::ImageBufferRemoteIOSurfaceBackend::draw(WebCore::GraphicsContext&, WebCore::FloatRect const&, WebCore::FloatRect const&, WebCore::ImagePaintingOptions const&) + 36 (ImageBufferRemoteIOSurfaceBackend.cpp:113)
2   com.apple.WebCore             	0x000000011c110440 drawImageToContext + 28 (CanvasRenderingContext2DBase.cpp:1818) [inlined]
3   com.apple.WebCore             	0x000000011c110440 void WebCore::CanvasRenderingContext2DBase::fullCanvasCompositedDrawImage<WebCore::ImageBuffer>(WebCore::ImageBuffer&, WebCore::FloatRect const&, WebCore::FloatRect const&, WebCore::CompositeOperator) + 660 (CanvasRenderingContext2DBase.cpp:1847)
4   com.apple.WebCore             	0x000000011c10fe80 WebCore::CanvasRenderingContext2DBase::drawImage(WebCore::CanvasBase&, WebCore::FloatRect const&, WebCore::FloatRect const&) + 524 (CanvasRenderingContext2DBase.cpp:1610)
5   com.apple.WebCore             	0x000000011c1150ac operator()<WTF::RefPtr<WebCore::HTMLCanvasElement> > + 76 (CanvasRenderingContext2DBase.cpp:1431) [inlined]
6   com.apple.WebCore             	0x000000011c1150ac __invoke_constexpr<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)>, WTF::RefPtr<WebCore::HTMLCanvasElement> &> + 76 (type_traits:3700) [inlined]
7   com.apple.WebCore             	0x000000011c1150ac operator()<std::__variant_detail::__alt<1, WTF::RefPtr<WebCore::HTMLCanvasElement> > &> + 76 (variant:615) [inlined]
8   com.apple.WebCore             	0x000000011c1150ac __invoke_constexpr<std::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)> >, std::__variant_detail::__alt<1, WTF::RefPtr<WebCore::HTMLCanvasElement> > &> + 76 (type_traits:3700) [inlined]
9   com.apple.WebCore             	0x000000011c1150ac decltype(auto) std::__1::__variant_detail::__visitation::__base::__dispatcher<1ul>::__dispatch<std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebCore::CanvasRenderingContext2DBase::drawImage(std::__1::variant<WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&&, float, float)::$_0, WebCore::CanvasRenderingContext2DBase::drawImage(std::__1::variant<WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&&, float, float)::$_1> >&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&>(std::__1::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<WebCore::CanvasRenderingContext2DBase::drawImage(std::__1::variant<WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&&, float, float)::$_0, WebCore::CanvasRenderingContext2DBase::drawImage(std::__1::variant<WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&&, float, float)::$_1> >&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&) + 112 (variant:497)
10  com.apple.WebCore             	0x000000011c10eda0 __visit_alt<std::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)> >, std::__variant_detail::__impl<WTF::RefPtr<WebCore::HTMLImageElement>, WTF::RefPtr<WebCore::HTMLCanvasElement>, WTF::RefPtr<WebCore::ImageBitmap>, WTF::RefPtr<WebCore::CSSStyleImageValue>, WTF::RefPtr<WebCore::HTMLVideoElement> > &> + 20 (variant:460) [inlined]
11  com.apple.WebCore             	0x000000011c10eda0 __visit_alt<std::__variant_detail::__visitation::__variant::__value_visitor<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)> >, std::variant<WTF::RefPtr<WebCore::HTMLImageElement>, WTF::RefPtr<WebCore::HTMLCanvasElement>, WTF::RefPtr<WebCore::ImageBitmap>, WTF::RefPtr<WebCore::CSSStyleImageValue>, WTF::RefPtr<WebCore::HTMLVideoElement> > &> + 20 (variant:567) [inlined]
12  com.apple.WebCore             	0x000000011c10eda0 __visit_value<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)>, std::variant<WTF::RefPtr<WebCore::HTMLImageElement>, WTF::RefPtr<WebCore::HTMLCanvasElement>, WTF::RefPtr<WebCore::ImageBitmap>, WTF::RefPtr<WebCore::CSSStyleImageValue>, WTF::RefPtr<WebCore::HTMLVideoElement> > &> + 24 (variant:585) [inlined]
13  com.apple.WebCore             	0x000000011c10eda0 visit<WTF::Visitor<(lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)>, std::variant<WTF::RefPtr<WebCore::HTMLImageElement>, WTF::RefPtr<WebCore::HTMLCanvasElement>, WTF::RefPtr<WebCore::ImageBitmap>, WTF::RefPtr<WebCore::CSSStyleImageValue>, WTF::RefPtr<WebCore::HTMLVideoElement> > &> + 40 (variant:1654) [inlined]
14  com.apple.WebCore             	0x000000011c10eda0 switchOn<std::variant<WTF::RefPtr<WebCore::HTMLImageElement>, WTF::RefPtr<WebCore::HTMLCanvasElement>, WTF::RefPtr<WebCore::ImageBitmap>, WTF::RefPtr<WebCore::CSSStyleImageValue>, WTF::RefPtr<WebCore::HTMLVideoElement> > &, (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1424:9), (lambda at ./html/canvas/CanvasRenderingContext2DBase.cpp:1429:9)> + 56 (StdLibExtras.h:392) [inlined]
15  com.apple.WebCore             	0x000000011c10eda0 WebCore::CanvasRenderingContext2DBase::drawImage(std::__1::variant<WTF::RefPtr<WebCore::HTMLImageElement, WTF::RawPtrTraits<WebCore::HTMLImageElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLImageElement> >, WTF::RefPtr<WebCore::HTMLCanvasElement, WTF::RawPtrTraits<WebCore::HTMLCanvasElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLCanvasElement> >, WTF::RefPtr<WebCore::ImageBitmap, WTF::RawPtrTraits<WebCore::ImageBitmap>, WTF::DefaultRefDerefTraits<WebCore::ImageBitmap> >, WTF::RefPtr<WebCore::CSSStyleImageValue, WTF::RawPtrTraits<WebCore::CSSStyleImageValue>, WTF::DefaultRefDerefTraits<WebCore::CSSStyleImageValue> >, WTF::RefPtr<WebCore::HTMLVideoElement, WTF::RawPtrTraits<WebCore::HTMLVideoElement>, WTF::DefaultRefDerefTraits<WebCore::HTMLVideoElement> > >&&, float, float) + 76 (CanvasRenderingContext2DBase.cpp:1423)
16  com.apple.WebCore             	0x000000011b0292b8 operator() + 24 (JSCanvasRenderingContext2D.cpp:1826) [inlined]
17  com.apple.WebCore             	0x000000011b0292b8 toJS<WebCore::IDLUndefined, (lambda at /Volumes/Data/worker/ios-simulator-15-release/build/WebKitBuild/Release-iphonesimulator/DerivedSources/WebCore/JSCanvasRenderingContext2D.cpp:1826:5)> + 24 (JSDOMConvertBase.h:168) [inlined]
18  com.apple.WebCore             	0x000000011b0292b8 jsCanvasRenderingContext2DPrototypeFunction_drawImage1Body + 72 (JSCanvasRenderingContext2D.cpp:1826) [inlined]
19  com.apple.WebCore             	0x000000011b0292b8 jsCanvasRenderingContext2DPrototypeFunction_drawImageOverloadDispatcher + 624 (JSCanvasRenderingContext2D.cpp:1903) [inlined]
20  com.apple.WebCore             	0x000000011b0292b8 call<&WebCore::jsCanvasRenderingContext2DPrototypeFunction_drawImageOverloadDispatcher, WebCore::CastedThisErrorBehavior::Throw> + 624 (JSDOMOperation.h:63) [inlined]
21  com.apple.WebCore             	0x000000011b0292b8 WebCore::jsCanvasRenderingContext2DPrototypeFunction_drawImage(JSC::JSGlobalObject*, JSC::CallFrame*) + 1036 (JSCanvasRenderingContext2D.cpp:1916)
Comment 1 Said Abou-Hallawa 2022-02-27 15:55:19 PST
Created attachment 453357 [details]
Patch
Comment 2 Simon Fraser (smfr) 2022-02-28 10:26:45 PST
Comment on attachment 453357 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:1833
> -    auto buffer = createCompositingBuffer(bufferRect);
> -    if (!buffer)
> -        return;
> -
>      auto* c = drawingContext();
>      if (!c)
>          return;
>  
> +    auto buffer = c->createImageBuffer(bufferRect.size());
> +    if (!buffer)
> +        return;

I'm confused by this ordering change. Doesn't drawingContext() need to get the context from the image buffer?
Comment 3 Said Abou-Hallawa 2022-02-28 13:01:24 PST
Comment on attachment 453357 [details]
Patch

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

>> Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:1833
>> +        return;
> 
> I'm confused by this ordering change. Doesn't drawingContext() need to get the context from the image buffer?

drawingContext() returns the GraphicsContext of CanvasBase::m_imageBuffer which must have been created before entering this function.

But 'buffer' is just a scratch ImageBuffer that we need to create for compositing the image to the drawingContext(). The fix just made it compatible with drawingContext(). In fact the reordering should not have any effect even without the new way to create the 'buffer'.
Comment 4 Said Abou-Hallawa 2022-03-02 12:08:12 PST
*** Bug 236929 has been marked as a duplicate of this bug. ***
Comment 5 Said Abou-Hallawa 2022-03-02 12:09:30 PST
rdar://89196918
Comment 6 Said Abou-Hallawa 2022-03-02 12:14:27 PST
Created attachment 453647 [details]
Patch
Comment 7 EWS 2022-03-04 11:41:51 PST
Committed r290839 (248075@main): <https://commits.webkit.org/248075@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453647 [details].