Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FF113 supports WebGPU on nightly #19370

Merged
merged 17 commits into from
Apr 21, 2023
Merged

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Apr 14, 2023

FF113 supports WebGPU on nightly in https://bugzilla.mozilla.org/show_bug.cgi?id=1746245

Spec has iterated a lots since this was defined. Probably not worth tracking introduction of various parts behind pref. This just tracks broad WebGPU support as Preview.

Open questions on devs here: https://bugzilla.mozilla.org/show_bug.cgi?id=1746245#c21

Other docs work can be tracked in mdn/content#26157


List is what is documented and in BCD. X indicates also in WebIdl for FF. So this is check of coverage.

  • GPU
  • GPUAdapter
  • GPUAdapterInfo
  • GPUBindGroup
  • GPUBindGroupLayout
  • GPUBuffer
  • GPUCanvasContext
  • GPUCommandBuffer
  • GPUCommandEncoder
  • GPUCompilationInfo
  • GPUCompilationMessage
  • GPUComputePassEncoder - behind pref dispatchWorkgroupsIndirect
  • GPUComputePipeline - not yet serializable
  • GPUDevice - note, not support createQuerySet()
  • GPUDeviceLostInfo
  • GPUError - not in IDL or visible on console.
  • GPUExternalTexture - not in IDL or visible on console.
  • GPUInternalError - not in IDL or visible on console.
  • GPUOutOfMemoryError (constructor commented out in IDL but seems to inherit one)
  • GPUPipelineError - not in IDL or visible on console.
  • GPUPipelineLayout
  • GPUQuerySet
  • GPUQueue - IDL comment out onSubmittedWorkDone()
  • GPURenderBundle
  • GPURenderBundleEncoder - Behind pref is drawIndirect(), drawIndexedIndirect().
  • GPURenderPassEncoder - IDL comments out beginOcclusionQuery, endOcclusionQuery, beginPipelineStatisticsQuery, endPipelineStatisticsQuery(), writeTimestamp(). Also behind pref is drawIndirect(), drawIndexedIndirect().
  • GPURenderPipeline - not yet serializable
  • GPUSampler
  • GPUShaderModule
  • GPUSupportedFeatures
  • GPUSupportedLimits
  • GPUTexture
  • GPUTextureView
  • GPUUncapturedErrorEvent
  • GPUValidationError - oddly the message is shown as in IDL for FF (not inherited) but as inherited in spec.
  • HTMLCanvasElement.getContext()
  • Navigator.gpu
  • WorkerNavigator.gpu - NOT supported - so you can't get a handle on this in workers on FF

A few issues:

  • GPUComputePipeline and GPUrenderPipeline are serializable but that isn't supported yet. I don't think we need to track that in preview right? i.e. I assume when this goes to release it will be serializable and that is the only thing that matters. Of course hard to remember that needs to be confirmed.

@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Apr 14, 2023
api/GPU.json Outdated Show resolved Hide resolved
@@ -50,9 +54,13 @@
},
"edge": "mirror",
"firefox": {
"version_added": "preview",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This count and type are in a dictionary GPUQuerySetDescriptor, and it is not so obvious these become properties of GPUQuerySet. Question is, if these are properties, why is pipelineStatistics not present - should it be?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are you getting pipelineStatistics from? I can't see it in the editor's draft anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisdavidmills It's probably not. To work out what is implemented and not I've been working through the FF WebIDL: https://searchfox.org/mozilla-central/source/dom/webidl/WebGPU.webidl

Confusingly this is close, but not the same as the spec IDL. So you get oddities like inheritance of errors not being the same, and so on. If it isn't in the spec it might just be useful for Mozilla, or it might reflect and older spec. I linked to the bugzilla bug above with some questions, and I hope the developers will comment.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, cool. I checked through the Chromium IDL, and it's not in there either.

@hamishwillee
Copy link
Collaborator Author

Thanks @Elchi3 - I've updated all the firefox-android records to false. I've also updated everything else I can see is in the Firefox IDL. I'm still waiting on confirmation of some things - IDL sometimes seems a bit magic to me, and the FF IDL doesn't seem to always match spec IDL.

A few things though

  1. This is enabled by default in nightly in FF113, which is what I have recorded.
    • I know it is behind a pref since before FF70 but not clear the precise version, or what versions various features were added. I might get that info from the dev team, but I might not. Now this is in preview, is it worth chasing this information about the pref?
    • I'd rather not - from what I can tell if you want to play with this you can/should take nightly.
  2. Some of the items are serializable in spec but not in FF. I am assuming that this will be fixed for release, so am not adding serializable subfeature to track those.

@chrisdavidmills
Copy link
Collaborator

know it is behind a pref since before FF70 but not clear the precise version, or what versions various features were added. I might get that info from the dev team, but I might not. Now this is in preview, is it worth chasing this information about the pref?

@hamishwillee I don't think this would be very helpful anyway. The spec has changed quite a bit since then.

@chrisdavidmills
Copy link
Collaborator

Some of the items are serializable in spec but not in FF. I am assuming that this will be fixed for release, so am not adding serializable subfeature to track those.

Should I be doing that for Chrome?

@hamishwillee
Copy link
Collaborator Author

Some of the items are serializable in spec but not in FF. I am assuming that this will be fixed for release, so am not adding serializable subfeature to track those.

Should I be doing that for Chrome?

If chrome didn't implement all the interfaces then realistically I think you'd want to add a serializable subfeature to track what is and is not supported. But I am not sure whether it should be added to the ones that do support the feature or do not, or both!

We need advice from @Elchi3 or @queengooborg

@hamishwillee hamishwillee marked this pull request as ready for review April 18, 2023 02:49
@hamishwillee
Copy link
Collaborator Author

Hi @Elchi3 , @chrisdavidmills

I've done some more research/testing today and update this appropriately. It is now as far as I can tell accurate and ready for review.

Note however that it may not yet be complete w.r.t. behaviour features.
Specifically, I THINK from the data guidelines for APIs that we should record the support status for behaviour features, whether or not they are supported or have different compatibility in any browser: https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/api.md#secure-context-required-secure_context_required
If I am correct we need to add secure_context_required and worker_support to any marked in the IDL as such.
Further, serialization is also a (newer) behaviour but there is no data guideline. We should add one, and it should match the logic for the others. @Elchi3 can you confirm the rule?

Even so I would prefer we merged this and then added the behaviours as a follow on - OK?

@Elchi3
Copy link
Member

Elchi3 commented Apr 18, 2023

If I am correct we need to add secure_context_required and worker_support to any marked in the IDL as such.

This has not been my practice. There are many interfaces in BCD that don't have secure_context_required because the version information would be the same as the basic feature itself. Only if the https requirement came later (after the feature shipped initially without this requirement), it becomes a case for BCD and you add a secure_context_required sub feature.

I opened openwebdocs/project#146 how to record security information in the front-matter as I don't want the BCD project to become a general sink for all kinds of data. It does a good job for compatibility data but I think general data should live more closely to the content (or should come from the specs via webref directly).

(Same for workers, but there isn't just one type of workers and someone should write up a proposal how to systematically record worker information analog to my security proposal, but let's maybe get one thing done, learn, and then do more of this)

Even so I would prefer we merged this and then added the behaviours as a follow on - OK?

Yes.

@Elchi3
Copy link
Member

Elchi3 commented Apr 18, 2023

Further, serialization is also a (newer) behaviour but there is no data guideline. We should add one, and it should match the logic for the others. @Elchi3 can you confirm the rule?

I'm not quite sure I have the context here. Happy to review data and work on a guideline as a followup. Do you have pointers for me?

api/GPU.json Outdated Show resolved Hide resolved
@hamishwillee
Copy link
Collaborator Author

@Elchi3 2. W.r.t. secure_context, fair enough.

I'm not quite sure I have the context here. Happy to review data and work on a guideline as a followup. Do you have pointers for me?

So a serializable object is one that you can send in a postMessage or use in a structured clone. The list of things to which it applies is provided in Structured clone algorithm > Supported types. When I added Errors to that list, I also made sure their descriptions mentioned this feature (but did not add to others):

image

The data rule is the same as for secure content - you add it if there is a compatibility change. That was the case for the errors - see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/EvalError#browser_compatibility

Is that enough?

MDN docs side I think we probably should add metadata for serializable, secure context, etc by default and use icons or whatever to indicate these "features".

@hamishwillee
Copy link
Collaborator Author

@Elchi3 I removed the bugzilla link (agree not useful). Serialization has not been implemented on anything yet as far as we can tell, so there is nothing to record there. Secure context has been implemented as far as we can tell so compatibility issue to record.

Upshot, I think this is good and ready to go in. Can you review/merge?

@Elchi3
Copy link
Member

Elchi3 commented Apr 21, 2023

Thanks Hamish! This all sounds great to me. Thanks for the info on serialization. Would be nice to formalize this in a guideline (on both BCD and MDN). How it is done for EvalError looks good to me.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@Elchi3 Elchi3 merged commit 07e913b into mdn:main Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants