686732 - Implement minimal-capabilities WebGL mode

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Would be nice to allow webdevs to ensure that their WebGL page will work everywhere.

In this mode all capabilities like MAX_TEXTURE_SIZE should be set to the minimum allowed value as per the spec,
http://www.khronos.org/registry/gles/specs/2.0/es_full_spec_2.0.25.pdf
section 6.2 State Tables.

Also this mode should disable all WebGL extensions.

The code to change is in content/canvas/src, especially InitAndValidateGL in WebGLContextValidate.cpp, and the patch would do something like

    PRBool restricted_minimal_mode =
        Preferences::GetBool("webgl.restricted_minimal_mode", PR_FALSE);

    if (restricted_minimal_mode) {
       mGLMaxTextureSize = 64;
    } else {
       gl->fGetIntegerv(LOCAL_GL_MAX_TEXTURE_SIZE, &mGLMaxTextureSize);        
    }

Also, to disable extensions, you could edit WebGLContext::GetExtension, I think if you return NS_OK right away with *retval=null then it will prevent any extension from getting enabled. And you want to edit WebGLContext::GetSupportedExtensions too to avoid advertising any extension.

Comment 1

[Christoph Diehl [:posidron]]

Attachment: Implement minimum capability mode in WebGL

Implements minimum capability mode in WebGL.

Benoit, latest review + changes compiled without errors but please verify again and let me know what you think and what could be changed/added. :-)

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Sorry for the long delay replying. Your patch is really perfect by itself, but I realize now that some of the min values from the spec are so low that all pages are going to fail in this mode, so I now think that instead of sticking stricly to the min values from the spec, we should sometimes instead follow what is supported by all the hardware around. Specifically, I think that these three min values:

+#define MINVALUE_GL_MAX_TEXTURE_SIZE                  64    // Page 162
+#define MINVALUE_GL_MAX_CUBE_MAP_TEXTURE_SIZE         16    // Page 162
+#define MINVALUE_GL_MAX_RENDERBUFFER_SIZE             1     // Page 164

are the ones that we need to change to match the real world. I propose these values instead:

+#define MINVALUE_GL_MAX_TEXTURE_SIZE                  1024
+#define MINVALUE_GL_MAX_CUBE_MAP_TEXTURE_SIZE         512
+#define MINVALUE_GL_MAX_RENDERBUFFER_SIZE             1024

Please update the comments to explain what the min values from the spec are (64, 16, 1) and why we're not using them (they're too stupidly low, all the hardware supports much higher values).

Comment 3

[Christoph Diehl [:posidron]]

Attachment: Implement minimum capability mode in WebGL

(In reply to Benoit Jacob [:bjacob] from comment #2)
> +#define MINVALUE_GL_MAX_TEXTURE_SIZE                  1024
> +#define MINVALUE_GL_MAX_CUBE_MAP_TEXTURE_SIZE         512
> +#define MINVALUE_GL_MAX_RENDERBUFFER_SIZE             1024

I have attached a new version with your recommendations. 

> Please update the comments to explain what the min values from the spec are
> (64, 16, 1) and why we're not using them (they're too stupidly low, all the
> hardware supports much higher values).

Those are the minimum values of TEXTURE_SIZE, CUBE_MAP_TEXTURE_SIZE, RENDERBUFFER_SIZE, which are defined in the specification on page 162 and 164.

Comment 4

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 564258 [details] [diff] [review]
Implement minimum capability mode in WebGL

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

Looks good, thanks!

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 564258 [details] [diff] [review]
Implement minimum capability mode in WebGL

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

I've been too quick to review this! It had a really nasty typo. Thankfully a conformance test caught it.

::: content/canvas/src/WebGLContextValidate.cpp
@@ +609,5 @@
> +            // On the public_webgl list, "problematic GetParameter pnames" thread, the following formula was given:
> +            //   mGLMaxVaryingVectors = min (GL_MAX_VERTEX_OUTPUT_COMPONENTS, GL_MAX_FRAGMENT_INPUT_COMPONENTS) / 4
> +            GLint maxVertexOutputComponents,
> +                  minFragmentInputComponents;
> +            gl->fGetIntegerv(LOCAL_GL_MAX_VERTEX_OUTPUT_COMPONENTS, &mGLMaxVertexUniformVectors);

This typo here (mGLMaxVertexUniformVectors instead of maxVertexOutputComponents) took me a while to debug! it was causing failures in gl-get-calls.html.

Comment 6

[Christoph Diehl [:posidron]]

Attachment: Implement minimum capability mode in WebGL

Corrected in the following patch. I have no idea why I touched this code, I am sorry!

Comment 7

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 566727 [details] [diff] [review]
Implement minimum capability mode in WebGL

This is on tryserver already:

https://tbpl.mozilla.org/?tree=Try&rev=ce4f36d45d7a

Comment 8

[Benoit Jacob [:bjacob] (mostly away)]

New try: https://tbpl.mozilla.org/?tree=Try&rev=5d7d6387dec0

Comment 9

[Benoit Jacob [:bjacob] (mostly away)]

pushed to inbound:

http://hg.mozilla.org/integration/mozilla-inbound/rev/7170a4b7a58d

congrats!

Comment 10

[Christoph Diehl [:posidron]]

Wow, this is very exiting! Thanks for your support Benoit!

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7170a4b7a58d

Comment 12

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/WebGL#WebGL_debugging_and_testing

Firefox 10 for developers updated.