Closed Bug 1084514 Opened 8 years ago Closed 8 years ago

AndroidSurfaceTexture missing namespace


(Core :: Graphics, defect)

Not set





(Reporter: rillian, Assigned: snorp)




(2 files, 1 obsolete file)

Bug 1014614 seems to have broken the non-unified source builds on Android:

build/gfx/gl/AndroidSurfaceTexture.cpp:24:22: error: 'AndroidSurfaceTexture' was not declared in this scope
build/gfx/gl/AndroidSurfaceTexture.cpp:24:22: note: suggested alternative:
In file included from build/gfx/gl/AndroidSurfaceTexture.cpp:12:0:
build/gfx/gl/AndroidSurfaceTexture.h:38:7: note:   'mozilla::gl::AndroidSurfaceTexture'
Assignee: nobody → giles
Attachment #8507097 - Flags: review?(jgilbert)
Comment on attachment 8507097 [details] [diff] [review]
Fix namespace issues with the non-unified build

Stealing review. Just one nit:

>diff --git a/gfx/gl/GLBlitHelper.cpp b/gfx/gl/GLBlitHelper.cpp
>index ff85b78..da5de96 100644
>--- a/gfx/gl/GLBlitHelper.cpp
>+++ b/gfx/gl/GLBlitHelper.cpp
>@@ -4,6 +4,7 @@
>  * License, v. 2.0. If a copy of the MPL was not distributed with this
>  * file, You can obtain one at */
>+#include "AndroidSurfaceTexture.h"
> #include "GLBlitHelper.h"

We have a general rule that Foo.cpp should have Foo.h as its first #include. (as a sanity-check that Foo.h includes/declares everything that it actually needs)

So, GLBlitHelper.h needs to stay at the top here. Please insert the new include somewhere below that.

>@@ -738,7 +739,7 @@ GLBlitHelper::BlitSurfaceTextureImage(layers::SurfaceTextureImage* stImage)
>     surfaceTexture->UpdateTexImage();
>-    Matrix4x4 transform;
>+    gfx::Matrix4x4 transform;

(sanity-check note to self: we do use "gfx::" namespace-prefixing on other types used in this file, so this seems fine.)

r=me with the include reordering.
Attachment #8507097 - Flags: review?(jgilbert) → review+
OS: Linux → Android
Hardware: x86_64 → All
Updated patch to put the own header first. Carrying forward r=dholbert.
Attachment #8507097 - Attachment is obsolete: true
I support dholbert's review.
I busted a whole bunch of stuff! This patch fixes all of them.
I've already landed some fixes. You should rebase on top of current inbound changes from this bug and bug 1084607 and ask for review, to fix it the way you want.
Assignee: giles → snorp
I fixed up the namespace stuff in the relanding, so we can close this.
Closed: 8 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.