744543 - LayerManagerOGL doesn't build on Linux/clang

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[George Wright (:gw280) (needinfo me!)]

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
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Comment 1

[George Wright (:gw280) (needinfo me!)]

Attachment: Bug 744543 - LayerManagerOGL doesn't build on Linux/clang. r=?

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

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.

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

Comment 4

[George Wright (:gw280) (needinfo me!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bd071b2e8a16

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

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

Comment 6

[Kelsey Gilbert [:jgilbert]]

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.

Comment 7

[Kelsey Gilbert [:jgilbert]]

Alas, I am too late, as you already hit inbound with it.

Comment 8

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/bd071b2e8a16