Bug 200752 – WebGPU layers don't show up sometimes

• Component: New Bugs
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

WebGPU layers don't show up sometimes

[Simon Fraser (smfr)]

Created attachment 376343 [details]
Patch

[Simon Fraser (smfr)]

<rdar://problem/54279821>

[Simon Fraser (smfr)]

The patch needs to apply, and needs some tests.

[Justin Fan]

Have patch that applies and fix does work but working on making a simple test case to expose the original issue.

[Justin Fan]

Created attachment 376717 [details]
Patch

[Justin Fan]

Created attachment 376719 [details]
Patch

[Dean Jackson]

Comment on attachment 376719 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Reproduction of and writing a test for original issue is non-trivial. 
> +        Deferring test creation in favor of higher priority WebGPU efforts.
> +        Normal WebGPU behavior unaffected and covered by existing tests.

Remove this. You'll just have to live with the shame and not give excuses :)

> Source/WebCore/html/canvas/GPUBasedCanvasRenderingContext.cpp:53
> +        if (renderBox && renderBox->hasAcceleratedCompositing())

We might even want to ASSERT(renderBox->hasAcceleratedCompositing())

> Source/WebCore/rendering/RenderLayerBacking.cpp:2515
>  #if ENABLE(WEBGL) || ENABLE(ACCELERATED_2D_CANVAS)

Add || ENABLE(WEBGPU)

[Justin Fan]

(In reply to Dean Jackson from comment #7)
> Comment on attachment 376719 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=376719&action=review
> 
> > Source/WebCore/ChangeLog:12
> > +        Reproduction of and writing a test for original issue is non-trivial. 
> > +        Deferring test creation in favor of higher priority WebGPU efforts.
> > +        Normal WebGPU behavior unaffected and covered by existing tests.
> 
> Remove this. You'll just have to live with the shame and not give excuses :)
> 
> > Source/WebCore/rendering/RenderLayerBacking.cpp:2515
> >  #if ENABLE(WEBGL) || ENABLE(ACCELERATED_2D_CANVAS)
> 
> Add || ENABLE(WEBGPU)

Addressed! tyvm

[Justin Fan]

Created attachment 376720 [details]
Patch for landing

[Justin Fan]

Created attachment 376721 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 376721 [details]
Patch for landing

Clearing flags on attachment: 376721

Committed r248879: <https://trac.webkit.org/changeset/248879>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 376721 [details]
Patch for landing

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

> Source/WebCore/html/canvas/GPUBasedCanvasRenderingContext.cpp:39
> +GPUBasedCanvasRenderingContext::GPUBasedCanvasRenderingContext(CanvasBase& canvas)
> +    : CanvasRenderingContext(canvas)
> +    , ActiveDOMObject(canvas.scriptExecutionContext())
> +{
> +}

What is the rationale behind making this no longer inlined?

> Source/WebCore/html/canvas/GPUBasedCanvasRenderingContext.h:56
> +    GPUBasedCanvasRenderingContext(CanvasBase&);

This should be marked explicit (not a new thing).

[Justin Fan]

Comment on attachment 376721 [details]
Patch for landing

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

>> Source/WebCore/html/canvas/GPUBasedCanvasRenderingContext.cpp:39
>> +}
> 
> What is the rationale behind making this no longer inlined?

I wasn't the one who first made this change, but the way I understand it, unless there's a measurable run-time performance benefit, putting implementation of class methods in .cpp is safer for better compile behavior. Before this patch there was no cpp file for the header.

>> Source/WebCore/html/canvas/GPUBasedCanvasRenderingContext.h:56
>> +    GPUBasedCanvasRenderingContext(CanvasBase&);
> 
> This should be marked explicit (not a new thing).

Addressing!

[Justin Fan]

Reopening to attach new patch.

[Justin Fan]

Created attachment 376788 [details]
Patch

[Justin Fan]

Comment on attachment 376788 [details]
Patch

Clearing flags on attachment: 376788

Committed r248915: <https://trac.webkit.org/changeset/248915>

[Justin Fan]

All reviewed patches have been landed.  Closing bug.