Closed Bug 1084514 Opened 10 years ago Closed 10 years ago

AndroidSurfaceTexture missing namespace

Categories

(Core :: Graphics, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: rillian, Assigned: snorp)

References

Details

Attachments

(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 http://mozilla.org/MPL/2.0/. */
> 
>+#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.

https://hg.mozilla.org/integration/mozilla-inbound/rev/869a28961b5c
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.
I had to back out bug 1014614 for bustage, and this was getting in my way, so it's backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3f27d263d0f7
Assignee: giles → snorp
I fixed up the namespace stuff in the relanding, so we can close this.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.