Last Comment Bug 879304 - WebGL's 32 warnings per context limit should be configurable by a preference
: WebGL's 32 warnings per context limit should be configurable by a preference
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla24
Assigned To: Guillaume Abadie
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-04 08:58 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2013-06-11 14:47 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
bug 879304 fix : adding prefrence var "webgl.max-warnings-per-context" (2.87 KB, patch)
2013-06-04 11:44 PDT, Guillaume Abadie
jacob.benoit.1: review+
Details | Diff | Splinter Review
patch for landing (2.87 KB, patch)
2013-06-04 11:51 PDT, Guillaume Abadie
guillaume.abadie: review+
Details | Diff | Splinter Review
patch with correct usage of Preference API (2.87 KB, patch)
2013-06-04 12:41 PDT, Guillaume Abadie
jacob.benoit.1: review+
ryanvm: checkin+
Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2013-06-04 08:58:49 PDT
Generating JS warnings is slow. WebGL scenes that generate many can be slow because of that. For that reason, we have placed a limit of 32 warnings per WebGL context, after which we stop generating WebGL warnings.

This is in content/canvas/src/WebGLContext.h:

    bool ShouldGenerateWarnings() const {
        return mAlreadyGeneratedWarnings < 32;
    }

This '32' here should be configurable by a preference (something in about:config).

Preferences can be accessed like we do in WebGLContext::SetDimensions in WebGLContext.cpp (search for "webgl.force-enabled"). Please add a new integer preference, webgl.max-warnings-per-context, defaulting to 32, and use it there.

Adding new preferences is done by editing this file:

  modules/libpref/src/init/all.js
Comment 1 Guillaume Abadie 2013-06-04 11:44:49 PDT
Created attachment 758072 [details] [diff] [review]
bug 879304 fix : adding prefrence var "webgl.max-warnings-per-context"
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2013-06-04 11:47:38 PDT
Comment on attachment 758072 [details] [diff] [review]
bug 879304 fix : adding prefrence var "webgl.max-warnings-per-context"

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

r+ with a couple of style nits

::: content/canvas/src/WebGLContext.cpp
@@ +189,5 @@
>  
>      mAlreadyGeneratedWarnings = 0;
>      mAlreadyWarnedAboutFakeVertexAttrib0 = false;
> +    Preferences::GetInt("webgl.max-warnings-per-context",&mMaxWarnings);
> +    if ( mMaxWarnings < -1 )

style

::: content/canvas/src/WebGLContext.h
@@ +1140,5 @@
>      bool ShouldGenerateWarnings() const {
> +        if (mMaxWarnings == -1) {
> +            return true;
> +        }
> +        

trailing whitespace
Comment 3 Guillaume Abadie 2013-06-04 11:51:57 PDT
Created attachment 758077 [details] [diff] [review]
patch for landing

carrying forward r+ from bjacob
Comment 4 Guillaume Abadie 2013-06-04 12:41:09 PDT
Created attachment 758111 [details] [diff] [review]
patch with correct usage of Preference API
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2013-06-04 12:41:41 PDT
Comment on attachment 758111 [details] [diff] [review]
patch with correct usage of Preference API

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

Thanks ;-)
Comment 6 Ryan VanderMeulen [:RyanVM] 2013-06-04 12:42:13 PDT
FYI, for a bug with a single patch, you can just use the checkin-needed keyword instead of setting checkin? on the individual patch.
Comment 7 Ryan VanderMeulen [:RyanVM] 2013-06-04 12:45:22 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8c2a4b9971c
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-06-04 17:51:04 PDT
https://hg.mozilla.org/mozilla-central/rev/f8c2a4b9971c

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