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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: bjacob, Assigned: guillaume.abadie)

Details

Attachments

(1 file, 2 obsolete files)

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 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+
Attached patch patch for landing (obsolete) — Splinter Review
carrying forward r+ from bjacob
Attachment #758077 - Flags: review+
Attachment #758077 - Flags: checkin?
Attachment #758072 - Attachment is obsolete: true
Attachment #758077 - Attachment is obsolete: true
Attachment #758077 - Flags: checkin?
Attachment #758111 - Flags: review?(bjacob)
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?
FYI, for a bug with a single patch, you can just use the checkin-needed keyword instead of setting checkin? on the individual patch.
Attachment #758111 - Flags: checkin? → checkin+
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.

Attachment

General

Created:
Updated:
Size: