Last Comment Bug 686732 - Implement minimal-capabilities WebGL mode
: Implement minimal-capabilities WebGL mode
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Christoph Diehl [:posidron]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-14 11:58 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2011-12-13 06:25 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Implement minimum capability mode in WebGL (16.08 KB, patch)
2011-09-15 15:34 PDT, Christoph Diehl [:posidron]
no flags Details | Diff | Splinter Review
Implement minimum capability mode in WebGL (16.17 KB, patch)
2011-10-03 10:54 PDT, Christoph Diehl [:posidron]
jacob.benoit.1: review+
Details | Diff | Splinter Review
Implement minimum capability mode in WebGL (16.17 KB, patch)
2011-10-12 19:39 PDT, Christoph Diehl [:posidron]
jacob.benoit.1: review+
Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2011-09-14 11:58:49 PDT
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] 2011-09-15 15:34:31 PDT
Created attachment 560471 [details] [diff] [review]
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) 2011-10-01 15:37:10 PDT
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] 2011-10-03 10:54:36 PDT
Created attachment 564258 [details] [diff] [review]
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) 2011-10-11 12:55:08 PDT
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) 2011-10-12 19:07:31 PDT
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] 2011-10-12 19:39:03 PDT
Created attachment 566727 [details] [diff] [review]
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) 2011-10-12 19:40:37 PDT
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) 2011-10-12 20:01:56 PDT
New try: https://tbpl.mozilla.org/?tree=Try&rev=5d7d6387dec0
Comment 9 Benoit Jacob [:bjacob] (mostly away) 2011-10-13 05:12:53 PDT
pushed to inbound:

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

congrats!
Comment 10 Christoph Diehl [:posidron] 2011-10-13 05:35:19 PDT
Wow, this is very exiting! Thanks for your support Benoit!
Comment 11 Ed Morley [:emorley] 2011-10-14 03:58:51 PDT
https://hg.mozilla.org/mozilla-central/rev/7170a4b7a58d
Comment 12 Eric Shepherd [:sheppy] 2011-12-13 06:25:06 PST
Documented:

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

Firefox 10 for developers updated.

Note You need to log in before you can comment on or make changes to this bug.