The default bug view has changed. See this FAQ.

AndroidSurfaceTexture missing namespace

RESOLVED INVALID

Status

()

Core
Graphics
RESOLVED INVALID
3 years ago
3 years ago

People

(Reporter: rillian, Assigned: snorp)

Tracking

Trunk
All
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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'
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Created attachment 8507097 [details] [diff] [review]
Fix namespace issues with the non-unified build
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
Created attachment 8507160 [details] [diff] [review]
Fix namespace issues with non-unified builds v2

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.
Created attachment 8507201 [details] [diff] [review]
namespace-fixes.diff

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
Last Resolved: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.