Open Bug 1072577 Opened 5 years ago Updated 9 months ago

WebGL - Add commit hash to ANGLE version string

Categories

(Core :: Canvas: WebGL, task, P5)

task

Tracking

()

People

(Reporter: wlitwinczyk, Assigned: wlitwinczyk)

References

Details

Attachments

(3 files, 6 obsolete files)

Add angle's commit hash to the version string so we know what version of angle is being used by FF.
Attached patch angle_version_string_patch (obsolete) — Splinter Review
Uploading, have yet to test on windows yet.
Attachment #8494764 - Flags: review?(jgilbert)
Attached patch angle_version_string_patch (obsolete) — Splinter Review
Missed include, but works on windows now
Attachment #8494764 - Attachment is obsolete: true
Attachment #8494764 - Flags: review?(jgilbert)
Attachment #8494779 - Flags: review?(jgilbert)
Attached image angle_version.png (obsolete) —
Attached patch angle_version_string_patch (obsolete) — Splinter Review
Added version string to renderer info in about:support instead.
Attachment #8494779 - Attachment is obsolete: true
Attachment #8494779 - Flags: review?(jgilbert)
Attachment #8494827 - Flags: review?(jgilbert)
Attached image angle_version.png
Attachment #8494780 - Attachment is obsolete: true
Comment on attachment 8494827 [details] [diff] [review]
angle_version_string_patch

Review of attachment 8494827 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the churn, but we can't do it this way.

::: dom/webidl/WebGLRenderingContext.webidl
@@ +812,5 @@
>  interface WebGLExtensionDebugRendererInfo
>  {
>      const GLenum UNMASKED_VENDOR_WEBGL        = 0x9245;
>      const GLenum UNMASKED_RENDERER_WEBGL      = 0x9246;
> +    const GLenum UNMASKED_VERSION_WEBGL       = 0x9247;

I think we actually need to make a new extension for this. :(
We definitely cannot assign a new enum value on our own.

I think at this point what we actually want is just:
any getUnmaskedParameter(GLenum);

Let's make a new MOZ_UNMASKED_GET extension.
Attachment #8494827 - Flags: review?(jgilbert) → review-
Just add it to the unmasked renderer string if it's ANGLE. Adding a new extension for this seems like huge overkill.
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #7)
> Just add it to the unmasked renderer string if it's ANGLE. Adding a new
> extension for this seems like huge overkill.

This isn't all we want though. We also want the unmasked extension string, and eventually a list of recognized driver errata and active workarounds. (unmasked max textures could be useful to know)

I don't think we should be afraid of making our own extensions for this stuff.
Will this new extension replace the existing one? Or add-on to it?
Attached patch unmasked_get_extension (obsolete) — Splinter Review
I think this is in the right direction. I mostly copied the debug extension.
Attachment #8494827 - Attachment is obsolete: true
Attachment #8495504 - Flags: review?(jgilbert)
Comment on attachment 8495504 [details] [diff] [review]
unmasked_get_extension

Review of attachment 8495504 [details] [diff] [review]:
-----------------------------------------------------------------

We decided to go for a new function instead of adding each enum.
Attachment #8495504 - Flags: review?(jgilbert) → review-
Attached patch unmasked_get_extension (obsolete) — Splinter Review
Attachment #8495504 - Attachment is obsolete: true
Attachment #8496122 - Flags: review?(jgilbert)
Update about:support page to include more information about the WebGL renderer.
Attachment #8496132 - Flags: review?(jgilbert)
Comment on attachment 8496132 [details] [diff] [review]
about_support_patch

Review of attachment 8496132 [details] [diff] [review]:
-----------------------------------------------------------------

r=jgilbert in-office
Attachment #8496132 - Flags: review?(jgilbert) → review+
Comment on attachment 8496122 [details] [diff] [review]
unmasked_get_extension

Review of attachment 8496122 [details] [diff] [review]:
-----------------------------------------------------------------

r=jgilbert locally

::: dom/canvas/WebGLContextState.cpp
@@ +160,5 @@
> +        uint32_t* params = new uint32_t[length];
> +        gl->fGetIntegerv(LOCAL_GL_COMPRESSED_TEXTURE_FORMATS, reinterpret_cast<GLint*>(params));
> +
> +        JSObject* obj = Uint32Array::Create(cx, this, length, params);
> +        delete [] params; // TODO TODO - Is this a problem!? - TODO TODO

(jgilbert) Use an MFBT type to get RAII.
Attachment #8496122 - Flags: review?(jgilbert) → review+
r=jgilbert carried forward

Using an MFBT type for RAII now (in GetUnmaskedParameter). Cleaned up WebGLContext interface by making GetUnmaskedParameter private and only allowed WebGLExtensionUnmaskedGet to access it.
Attachment #8496122 - Attachment is obsolete: true
Attachment #8496304 - Flags: review+
Adding depends on because this patch was made on top of the patches from Bug 1015561, so that bug should land before this one.
Depends on: 1015561
Type: defect → task
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.