938970 - Build gfx/layers in unified mode

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

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

Attachment: patch

Reviewer note: you'll want to check that the static functions that were defined in multiple cpp files, and that are unified by this patch, were actually identical... have lots of fun!

https://tbpl.mozilla.org/?tree=Try&rev=e38d2a88d0ea

R? Matt for gfx/layers code changes, Ehsan for build-system changes.

Comment 1

[[e-mail]]

ISurfaceAllocator.cpp error C2872: “SharedMemory”:  ambiguous symbol


d:\develop\mozilla\central\gfx\layers\d3d9\LayerManagerD3D9Shaders.h(56) : error
 C2370: “LayerQuadVS”: redefinition; different storage class

same to sLayerManagerCount 

d:/develop/mozilla/central/gfx/layers\d3d11/CompositorD3D11.cpp(29) : error C201
1: “mozilla::layers::Vertex”:“struct”type redefinition

Comment 2

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

https://tbpl.mozilla.org/?tree=Try&rev=dad24ee4d043

Comment 3

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

Re: comment 1: thanks for the findings, that means that my above try push in comment 2 is going to fail, here is another try push on windows that might succeed: it takes the affected windows-specific source files out of UNIFIED_SOURCES back into regular SOURCES.

https://tbpl.mozilla.org/?tree=Try&rev=83adf89b4c74

Comment 4

[[e-mail]]

(In reply to Benoit Jacob [:bjacob] from comment #3)
> Re: comment 1: thanks for the findings, that means that my above try push in
> comment 2 is going to fail, here is another try push on windows that might
> succeed: it takes the affected windows-specific source files out of
> UNIFIED_SOURCES back into regular SOURCES.
> 
> https://tbpl.mozilla.org/?tree=Try&rev=83adf89b4c74

I forget to mention that I locally remove the opengl code from building. So the try might secceed. But in future, as the code change, the problem can appear.

Comment 5

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

And another iteration on Mac:
https://tbpl.mozilla.org/?tree=Try&rev=38b22976f3dc

Comment 6

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

One more for the road on all platforms:
https://tbpl.mozilla.org/?tree=Try&rev=0d71800dab1c

Comment 7

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

We're actually still far away from being able to land. The Mac failures are serious:

In file included from Unified_cpp_gfx_layers1.cpp:15:
In file included from ../../../../gfx/layers/client/CompositableClient.cpp:11:
In file included from ../../dist/include/mozilla/layers/TextureClientOGL.h:17:
In file included from ../../dist/include/mozilla/gfx/MacIOSurface.h:10:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/QuartzCore.framework/Headers/QuartzCore.h:7:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/QuartzCore.framework/Headers/CoreVideo.h:1:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CoreVideo.h:20:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVReturn.h:21:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreVideo.framework/Headers/CVBase.h:26:
In file included from /Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreFoundation.framework/Headers/CFBase.h:48:
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:501:16: error: reference to 'Point' is ambiguous
typedef struct Point                    Point;
               ^
/Developer/SDKs/MacOSX10.6.sdk/System/Library/Frameworks/CoreServices.framework/Headers/../Frameworks/CarbonCore.framework/Headers/MacTypes.h:497:8: note: candidate found by name lookup is 'Point'
struct Point {
       ^
../../dist/include/mozilla/gfx/Point.h:64:34: note: candidate found by name lookup is 'mozilla::gfx::Point'
typedef PointTyped<UnknownUnits> Point;
                                 ^



The problem here is that MacIOSurface.h is included in a few headers: TextureClientOGL.h and TextureHostOGL.h and so this problem is not just contained in one or two cpp files.

I think that the right way to fix this is to minimize the number of cpp files that have to include MacIOSurface.h by stopping including it in headers. Then we can move these cpp files back into SOURCES (not UNIFIED_SOURCES).

Comment 8

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

New try, new hope:

https://tbpl.mozilla.org/?tree=Try&rev=5e58135fb405

Comment 9

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

woohoohoo!! green! Thanks a bunch to Josh&BenWa for the Mac builds and to zhoubcfan@ for the above tips!

Comment 10

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

Attachment: 5/5. Switch gfx/layers to UNIFIED_SOURCES

Carrying forward r=ehsan,mattwoodrow

Comment 11

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

Attachment: 3/5. Some d3d tweaks needed to build with UNIFIED_SOURCES

Comment 12

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

Attachment: 4/5. Some gralloc-related tweaks required to build with UNIFIED_SOURCES

Comment 13

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

Attachment: 1/5. Move MacIOSurfaceTexture* classes to separate files, minimize the number of cpp files including MacIOSurface.h

Indeed, Mac system headers define Size and Point types in the root namespace that collide with our mozilla::gfx:: types as soon as we do |using namespace mozilla::gfx;|.

Since we do that in a lot of files, the easiest solution for now is to just minimize the number of cpp files that have to include these Mac headers, and keep these files out of UNIFIED_SOURCES for now.

Comment 14

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

Attachment: 2/5. Add more namespace qualifiers in gfx/layers code to avoid ambiguous namespace resolution

Comment 15

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

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/116329598a64
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c60b3513c790
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e8931a0bb421
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/08add9705bc1
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/56045f7c969d

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/116329598a64
https://hg.mozilla.org/mozilla-central/rev/c60b3513c790
https://hg.mozilla.org/mozilla-central/rev/e8931a0bb421
https://hg.mozilla.org/mozilla-central/rev/08add9705bc1
https://hg.mozilla.org/mozilla-central/rev/56045f7c969d