Bug 193473 – [WebGPU] Update vertex-buffer-triangle-strip.html to actually use vertex buffer

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGPU

[Justin Fan]

[WebGPU] Update vertex-buffer-triangle-strip.html to actually use vertex buffer

[Justin Fan]

Created attachment 359231 [details]
Patch

[Darin Adler]

Comment on attachment 359231 [details]
Patch

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

> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:138
> +        // FIXME: Setting bufferIndex to an index that DNE causes a SIGABRT when calling [MTLDevice newRenderPipelineStateWithDescriptor]
> +        // that is not caught by BLOCK_OBJC_EXCEPTIONS.

I don’t think "DNE" is standard terminology. Writing out "does not exist" or "is out of bounds" would be clearer.

But this FIXME comment is not very clear. What does the comment mean? It seems to be an interesting bit of trivia; what should we be fixing?

It’s like a comment saying "// FIXME: Dereferencing a null pointer causes a SIGABRT that is not caught by BLOCK_OBJC_EXCEPTIONS." True, but how exactly is this relevant to the next line of code? Is it possible that inputSlot is an index that does not exist? If so, what can we do about it?

[Simon Fraser (smfr)]

Comment on attachment 359231 [details]
Patch

This patch has too many FIXMEs.

[Justin Fan]

Thanks for the comments! Instead of writing  in fixmes I'll create a new bug to address the issues I raised for myself.

[Radar WebKit Bug Importer]

<rdar://problem/47326063>

[Justin Fan]

Created attachment 359297 [details]
Patch

[Dean Jackson]

Comment on attachment 359297 [details]
Patch

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

> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:137
> -        mtlAttributeDesc.bufferIndex = attributes[i].shaderLocation;
> +        mtlAttributeDesc.bufferIndex = attributes[i].inputSlot;

You don't mention why you made this change. The bug title only mentions the HTML. What was wrong about it?

> LayoutTests/webgpu/vertex-buffer-triangle-strip.html:17
> +    float4 position [[attribute(0)]];

Can you label with both attribute(0) and position to avoid having two identical structs?

[Myles C. Maxfield]

Comment on attachment 359297 [details]
Patch

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

> LayoutTests/webgpu/vertex-buffer-triangle-strip.html:32
> +fragment float4 fragment_main(VertexOut v [[stage_in]])

Does this need to take any arguments at all?

[Justin Fan]

Created attachment 359336 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 359336 [details]
Patch for landing

Clearing flags on attachment: 359336

Committed r240101: <https://trac.webkit.org/changeset/240101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.