1072577 - WebGL - Add commit hash to ANGLE version string

Status: NEW

(Core :: Graphics: CanvasWebGL, task, P5)

Description

[Walter Litwinczyk [:walter]]

Add angle's commit hash to the version string so we know what version of angle is being used by FF.

Comment 1

[Walter Litwinczyk [:walter]]

Attachment: angle_version_string_patch

Uploading, have yet to test on windows yet.

Comment 2

[Walter Litwinczyk [:walter]]

Attachment: angle_version_string_patch

Missed include, but works on windows now

Comment 3

[Walter Litwinczyk [:walter]]

Attachment: angle_version.png

Comment 4

[Walter Litwinczyk [:walter]]

Attachment: angle_version_string_patch

Added version string to renderer info in about:support instead.

Comment 5

[Walter Litwinczyk [:walter]]

Attachment: angle_version.png

Comment 6

[Kelsey Gilbert [:jgilbert]]

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.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Just add it to the unmasked renderer string if it's ANGLE. Adding a new extension for this seems like huge overkill.

Comment 8

[Kelsey Gilbert [:jgilbert]]

(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.

Comment 9

[Walter Litwinczyk [:walter]]

Will this new extension replace the existing one? Or add-on to it?

Comment 10

[Walter Litwinczyk [:walter]]

Attachment: unmasked_get_extension

I think this is in the right direction. I mostly copied the debug extension.

Comment 11

[Kelsey Gilbert [: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.

Comment 12

[Walter Litwinczyk [:walter]]

Attachment: unmasked_get_extension

Comment 13

[Walter Litwinczyk [:walter]]

Attachment: about_support_patch

Update about:support page to include more information about the WebGL renderer.

Comment 14

[Walter Litwinczyk [:walter]]

Comment on attachment 8496132 [details] [diff] [review]
about_support_patch

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

r=jgilbert in-office

Comment 15

[Walter Litwinczyk [:walter]]

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.

Comment 16

[Walter Litwinczyk [:walter]]

Attachment: unmasked_get_extension

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.

Comment 17

[Walter Litwinczyk [:walter]]

Adding depends on because this patch was made on top of the patches from Bug 1015561, so that bug should land before this one.

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:jgilbert, could you have a look please?
For more information, please visit auto_nag documentation.