LayerManagerOGL doesn't build on Linux/clang

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gw280, Assigned: gw280)

Tracking

unspecified
mozilla14
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

We get this compile error:

/home/george/dev/mozilla-central/gfx/layers/opengl/LayerManagerOGL.cpp:265:7: error:
  non-constant-expression cannot be narrowed from type 'int' to 'GLenum' (aka 'unsigned
  int') in initializer list mGLContext->IsGLES2() ? LOCAL_GL_TEXTURE_RECTANGLE_ARB : 0
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Created attachment 614115 [details] [diff] [review]
Bug 744543 - LayerManagerOGL doesn't build on Linux/clang. r=?
Attachment #614115 - Flags: review?(bjacob)
Comment on attachment 614115 [details] [diff] [review]
Bug 744543 - LayerManagerOGL doesn't build on Linux/clang. r=?

Review of attachment 614115 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/LayerManagerOGL.cpp
@@ +266,5 @@
>      };
>  
> +    if (mGLContext->IsGLES2()) {
> +        textureTargets[1] = LOCAL_GL_TEXTURE_RECTANGLE_ARB;
> +    }

No need for braces, per coding style. They uselessly reduce risk of future bugs, which is an unfair advantage given to future developers over us.
Attachment #614115 - Flags: review?(bjacob) → review+
(In reply to Benoit Jacob [:bjacob] from comment #2)
> Comment on attachment 614115 [details] [diff] [review]
> Bug 744543 - LayerManagerOGL doesn't build on Linux/clang. r=?
> 
> Review of attachment 614115 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/opengl/LayerManagerOGL.cpp
> @@ +266,5 @@
> >      };
> >  
> > +    if (mGLContext->IsGLES2()) {
> > +        textureTargets[1] = LOCAL_GL_TEXTURE_RECTANGLE_ARB;
> > +    }
> 
> No need for braces, per coding style. They uselessly reduce risk of future
> bugs, which is an unfair advantage given to future developers over us.

The coding style disagrees, and wants braces.
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd071b2e8a16
(In reply to Ms2ger from comment #3)
> The coding style disagrees, and wants braces.

Indeed. But on the other hand, the MFBT coding style (mfbt/STYLE) doesn't want braces here. We need to harmonize this a bit...

Updated

5 years ago
Blocks: 574346
Comment on attachment 614115 [details] [diff] [review]
Bug 744543 - LayerManagerOGL doesn't build on Linux/clang. r=?

Review of attachment 614115 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/LayerManagerOGL.cpp
@@ +266,4 @@
>      };
>  
> +    if (mGLContext->IsGLES2()) {
> +        textureTargets[1] = LOCAL_GL_TEXTURE_RECTANGLE_ARB;

4-space indent in 2-space land.

@@ +273,5 @@
>  
>      for (PRUint32 i = 0; i < ArrayLength(textureTargets); i++) {
>        GLenum target = textureTargets[i];
>        if (!target)
>            continue;

If you don't mind fixing it, this should also be 2-space.
Alas, I am too late, as you already hit inbound with it.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/bd071b2e8a16
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.