Last Comment Bug 743753 - Always generate WebGL warnings (remove webgl.verbose pref) and other related improvements
: Always generate WebGL warnings (remove webgl.verbose pref) and other related ...
Status: RESOLVED FIXED
webgl-next
:
Product: Core
Classification: Components
Component: Canvas: WebGL (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
Mentors:
Depends on: 753734
Blocks: 599070
  Show dependency treegraph
 
Reported: 2012-04-09 11:34 PDT by Benoit Jacob [:bjacob] (mostly away)
Modified: 2012-05-24 10:53 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
1: limit the number of warnings per WebGL context (3.77 KB, patch)
2012-05-22 13:35 PDT, Benoit Jacob [:bjacob] (mostly away)
vladimir: review+
Details | Diff | Splinter Review
2: remove webgl.verbose pref (19.29 KB, patch)
2012-05-22 13:37 PDT, Benoit Jacob [:bjacob] (mostly away)
vladimir: review+
Details | Diff | Splinter Review
3: icing on the cake: rename LogMessage to GenerateWarning (25.43 KB, patch)
2012-05-22 13:39 PDT, Benoit Jacob [:bjacob] (mostly away)
vladimir: review+
Details | Diff | Splinter Review
4: cherry on the icing: fix the case issues in WebGL method names in warnings (31.97 KB, patch)
2012-05-22 13:42 PDT, Benoit Jacob [:bjacob] (mostly away)
vladimir: review+
Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2012-04-09 11:34:33 PDT
The reason why WebGL warnings are disabled by default (have to flip webgl.verbose pref) is that the last time I checked (1 year ago), pages generating many WebGL warnings could be considerably slowed down by them. It seemed that the cost of generating a JS warning was very high.

Two factors could eventually allow us to reconsider that:
 - if JS warnings become cheaper to generate (profile it!)
 - if most WebGL pages no longer generate many warnings (the worst offenders were pages relying on arbitrary attrib locations instead of calling {bind,get}AttribLocation.
Comment 1 Boris Zbarsky [:bz] 2012-04-09 11:53:55 PDT
If you use the nsContentUtils warning APIs, combined with the WarnOnce stuff on nsIDocument, assuming that's desirable here, then you could vastly reduce the number of warnings generated....
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2012-04-09 12:01:08 PDT
WarnOnce definitely sounds like something I want here. Thanks!
Comment 3 Benoit Jacob [:bjacob] (mostly away) 2012-05-22 13:33:56 PDT
The problem with ReportToConsole is that it wants a localized string. WebGL warnings aren't currently localized and while that might be a good idea, I don't want to block this on that.
Comment 4 Benoit Jacob [:bjacob] (mostly away) 2012-05-22 13:35:44 PDT
Created attachment 626153 [details] [diff] [review]
1: limit the number of warnings per WebGL context

Do not generate more than N (where N=32 currently) warnings per WebGL context. + informative extra warning when the Nth warning is generated.
Comment 5 Benoit Jacob [:bjacob] (mostly away) 2012-05-22 13:37:44 PDT
Created attachment 626157 [details] [diff] [review]
2: remove webgl.verbose pref

With patch 1, this pref isn't useful enough anymore to justify its existence.
Comment 6 Benoit Jacob [:bjacob] (mostly away) 2012-05-22 13:39:34 PDT
Created attachment 626158 [details] [diff] [review]
3: icing on the cake: rename LogMessage to GenerateWarning

This name matches more closely what this does and the 'severity' of these messages.
Comment 7 Benoit Jacob [:bjacob] (mostly away) 2012-05-22 13:42:40 PDT
Created attachment 626162 [details] [diff] [review]
4: cherry on the icing: fix the case issues in WebGL method names in warnings

Mostly a plain regex change. Some users reported they were puzzled by inconsistent case like TexImage2D vs texImage2D in WebGL warnings.

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