Closed
Bug 1084514
Opened 10 years ago
Closed 10 years ago
AndroidSurfaceTexture missing namespace
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: rillian, Assigned: snorp)
References
Details
Attachments
(2 files, 1 obsolete file)
2.33 KB,
patch
|
Details | Diff | Splinter Review | |
6.40 KB,
patch
|
Details | Diff | Splinter Review |
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 (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 3•10 years ago
|
||
Assignee: nobody → giles
Attachment #8507097 -
Flags: review?(jgilbert)
Comment 4•10 years ago
|
||
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+
Updated•10 years ago
|
OS: Linux → Android
Hardware: x86_64 → All
Reporter | ||
Comment 5•10 years ago
|
||
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
Comment 6•10 years ago
|
||
I support dholbert's review.
Assignee | ||
Comment 7•10 years ago
|
||
I busted a whole bunch of stuff! This patch fixes all of them.
Reporter | ||
Comment 8•10 years ago
|
||
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
Reporter | ||
Updated•10 years ago
|
Assignee: giles → snorp
Assignee | ||
Comment 10•10 years ago
|
||
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.
Description
•