Closed Bug 938970 Opened 6 years ago Closed 6 years ago

Build gfx/layers in unified mode

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: bjacob, Assigned: bjacob)

References

Details

(Whiteboard: [qa-])

Attachments

(6 files)

Attached patch patchSplinter Review
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.
Attachment #832709 - Flags: review?(matt.woodrow)
Attachment #832709 - Flags: review?(ehsan)
Attachment #832709 - Attachment is patch: true
Attachment #832709 - Flags: review?(ehsan) → review+
Attachment #832709 - Flags: review?(matt.woodrow) → review+
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
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
(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.
One more for the road on all platforms:
https://tbpl.mozilla.org/?tree=Try&rev=0d71800dab1c
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).
Assignee: nobody → bjacob
Blocks: unified
woohoohoo!! green! Thanks a bunch to Josh&BenWa for the Mac builds and to zhoubcfan@ for the above tips!
Carrying forward r=ehsan,mattwoodrow
Attachment #8333601 - Flags: review+
Attachment #8333604 - Flags: review?(bas) → review+
Attachment #8333603 - Flags: review?(bas) → review+
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.
Attachment #8333611 - Flags: review?(matt.woodrow)
Attachment #8333603 - Attachment description: 1/3. Some d3d tweaks needed to build with UNIFIED_SOURCES → 3/5. Some d3d tweaks needed to build with UNIFIED_SOURCES
Attachment #8333604 - Attachment description: 2/3. Some gralloc-related tweaks required to build with UNIFIED_SOURCES → 4/5. Some gralloc-related tweaks required to build with UNIFIED_SOURCES
Attachment #8333601 - Attachment description: 3/3. Switch gfx/layers to UNIFIED_SOURCES → 5/5. Switch gfx/layers to UNIFIED_SOURCES
Attachment #8333611 - Flags: review?(matt.woodrow) → review+
Attachment #8333612 - Flags: review?(matt.woodrow) → review+
Summary: Switch gfx/layers to UNIFIED_SOURCES → Build gfx/layers in unified mode
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.