Closed
Bug 879304
Opened 12 years ago
Closed 12 years ago
WebGL's 32 warnings per context limit should be configurable by a preference
Categories
(Core :: Graphics: CanvasWebGL, defect)
Core
Graphics: CanvasWebGL
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: bjacob, Assigned: guillaume.abadie)
Details
Attachments
(1 file, 2 obsolete files)
2.87 KB,
patch
|
bjacob
:
review+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #758072 -
Flags: review?(bjacob)
Reporter | ||
Comment 2•12 years ago
|
||
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
Attachment #758072 -
Flags: review?(bjacob) → review+
Assignee | ||
Comment 3•12 years ago
|
||
carrying forward r+ from bjacob
Attachment #758077 -
Flags: review+
Attachment #758077 -
Flags: checkin?
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #758072 -
Attachment is obsolete: true
Attachment #758077 -
Attachment is obsolete: true
Attachment #758077 -
Flags: checkin?
Attachment #758111 -
Flags: review?(bjacob)
Reporter | ||
Comment 5•12 years ago
|
||
Comment on attachment 758111 [details] [diff] [review]
patch with correct usage of Preference API
Review of attachment 758111 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks ;-)
Attachment #758111 -
Flags: review?(bjacob)
Attachment #758111 -
Flags: review+
Attachment #758111 -
Flags: checkin?
Comment 6•12 years ago
|
||
FYI, for a bug with a single patch, you can just use the checkin-needed keyword instead of setting checkin? on the individual patch.
Updated•12 years ago
|
Attachment #758111 -
Flags: checkin? → checkin+
Comment 7•12 years ago
|
||
Comment 8•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•