Last Comment Bug 717584 - FAIL_ON_WARNINGS in content/canvas/src
: FAIL_ON_WARNINGS in content/canvas/src
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Benoit Jacob [:bjacob] (mostly away)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-12 06:25 PST by Benoit Jacob [:bjacob] (mostly away)
Modified: 2012-01-16 20:01 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix content/canvas/src warnings (5.19 KB, patch)
2012-01-12 06:25 PST, Benoit Jacob [:bjacob] (mostly away)
no flags Details | Diff | Splinter Review
Fix unused-but-set warnings in content/html (1.22 KB, patch)
2012-01-12 06:27 PST, Benoit Jacob [:bjacob] (mostly away)
no flags Details | Diff | Splinter Review
Fix cast-to-ptr-of-different-size warning in nsCheapSets (693 bytes, patch)
2012-01-12 06:28 PST, Benoit Jacob [:bjacob] (mostly away)
no flags Details | Diff | Splinter Review
fix content/canvas/src warnings (6.97 KB, patch)
2012-01-12 11:29 PST, Benoit Jacob [:bjacob] (mostly away)
Ms2ger: review+
Details | Diff | Splinter Review

Description Benoit Jacob [:bjacob] (mostly away) 2012-01-12 06:25:20 PST
Created attachment 588013 [details] [diff] [review]
fix content/canvas/src warnings
Comment 1 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 06:27:41 PST
Created attachment 588014 [details] [diff] [review]
Fix unused-but-set warnings in content/html

I didn't feel like removing this rv altogether: it seems to contain some information about what we would want to check if we wanted to check for errors here, and the (void) rv serves as explicitly saying that we're not checking rv here.
Comment 2 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 06:28:55 PST
Created attachment 588015 [details] [diff] [review]
Fix cast-to-ptr-of-different-size warning in nsCheapSets

hg log is clueless about who owns this code (r1) so asking you again.
Comment 3 :Ms2ger (⌚ UTC+1/+2) 2012-01-12 07:03:03 PST
Comment on attachment 588013 [details] [diff] [review]
fix content/canvas/src warnings

>--- a/content/canvas/src/WebGLContext.cpp
>+++ b/content/canvas/src/WebGLContext.cpp
>+    (void) preferEGL;
>+    (void) useANGLE;

I'd prefer not doing the work to compute these if we aren't going to use them.

>--- a/content/canvas/src/WebGLContextGL.cpp
>+++ b/content/canvas/src/WebGLContextGL.cpp

What is the warning for the byteOffset changes?
Comment 4 :Ms2ger (⌚ UTC+1/+2) 2012-01-12 07:03:41 PST
Comment on attachment 588014 [details] [diff] [review]
Fix unused-but-set warnings in content/html

These are fixed in another bug
Comment 5 :Ms2ger (⌚ UTC+1/+2) 2012-01-12 07:04:38 PST
Comment on attachment 588015 [details] [diff] [review]
Fix cast-to-ptr-of-different-size warning in nsCheapSets

This too (bug 679832)
Comment 6 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 11:29:10 PST
Created attachment 588123 [details] [diff] [review]
fix content/canvas/src warnings

(In reply to Ms2ger from comment #3)
> Comment on attachment 588013 [details] [diff] [review]
> fix content/canvas/src warnings
> 
> >--- a/content/canvas/src/WebGLContext.cpp
> >+++ b/content/canvas/src/WebGLContext.cpp
> >+    (void) preferEGL;
> >+    (void) useANGLE;
> 
> I'd prefer not doing the work to compute these if we aren't going to use
> them.

OK, see new patch.

> 
> >--- a/content/canvas/src/WebGLContextGL.cpp
> >+++ b/content/canvas/src/WebGLContextGL.cpp
> 
> What is the warning for the byteOffset changes?

Cast to pointer from integer of different size.
Comment 7 :Ms2ger (⌚ UTC+1/+2) 2012-01-12 11:46:56 PST
Comment on attachment 588123 [details] [diff] [review]
fix content/canvas/src warnings

>+#ifdef XP_WIN
>     // allow forcing GL and not EGL/ANGLE
>     if (PR_GetEnv("MOZ_WEBGL_FORCE_OPENGL")) {
>         preferEGL = false;
>         useANGLE = false;
>         useOpenGL = true;
>     }
>+#endif

Don't ifdef the useOpenGL assignment, please.
Comment 8 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 12:15:28 PST
Why? useOpenGL is already initialized above.
Comment 9 :Ms2ger (⌚ UTC+1/+2) 2012-01-12 12:16:40 PST
It can be set to false in the previous block, no?
Comment 10 Benoit Jacob [:bjacob] (mostly away) 2012-01-12 12:36:30 PST
Ah yes, but then the only effect of that would be to circumvent blacklisting, and there exist much better ways to do that.

Actually, I don't really see the usefulness of this environment variable now that we seem to have prefs for everything. Jeff?
Comment 11 Jeff Gilbert [:jgilbert] 2012-01-13 01:51:35 PST
Agreed. By that virtue, why don't we eliminate the MOZ_WEBGL_PREFER_EGL EnvVar as well?
Comment 12 Benoit Jacob [:bjacob] (mostly away) 2012-01-16 14:09:23 PST
http://hg.mozilla.org/integration/mozilla-inbound/rev/5634697d0acd
Comment 13 Justin Wood (:Callek) 2012-01-16 20:01:01 PST
https://hg.mozilla.org/mozilla-central/rev/5634697d0acd

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