The default bug view has changed. See this FAQ.

Always generate WebGL warnings (remove webgl.verbose pref) and other related improvements

RESOLVED FIXED in mozilla15

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bjacob, Assigned: bjacob)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: webgl-next)

Attachments

(4 attachments)

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.
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....
(Assignee)

Comment 2

5 years ago
WarnOnce definitely sounds like something I want here. Thanks!
Blocks: 599070
(Assignee)

Updated

5 years ago
Depends on: 753734
(Assignee)

Updated

5 years ago
Summary: Always generate WebGL warnings (remove webgl.verbose pref) → Always generate WebGL warnings (remove webgl.verbose pref) and other related improvements
(Assignee)

Comment 3

5 years ago
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.
(Assignee)

Comment 4

5 years ago
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.
Attachment #626153 - Flags: review?(vladimir)
(Assignee)

Updated

5 years ago
Attachment #626153 - Attachment description: limit the number of warnings per WebGL context → 1: limit the number of warnings per WebGL context
(Assignee)

Comment 5

5 years ago
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.
Attachment #626157 - Flags: review?(vladimir)
(Assignee)

Comment 6

5 years ago
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.
Attachment #626158 - Flags: review?(vladimir)
(Assignee)

Comment 7

5 years ago
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.
Attachment #626162 - Flags: review?(vladimir)
Attachment #626153 - Flags: review?(vladimir) → review+
Attachment #626157 - Flags: review?(vladimir) → review+
Attachment #626158 - Flags: review?(vladimir) → review+
Attachment #626162 - Flags: review?(vladimir) → review+
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/69423dd75eb6
http://hg.mozilla.org/integration/mozilla-inbound/rev/6ac4c324163e
http://hg.mozilla.org/integration/mozilla-inbound/rev/b0dc5e163739
http://hg.mozilla.org/integration/mozilla-inbound/rev/434efc02a33b
Assignee: nobody → bjacob
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/69423dd75eb6
https://hg.mozilla.org/mozilla-central/rev/6ac4c324163e
https://hg.mozilla.org/mozilla-central/rev/b0dc5e163739
https://hg.mozilla.org/mozilla-central/rev/434efc02a33b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.