Closed Bug 560147 Opened 15 years ago Closed 15 years ago

Create unified GL Wrapper

Categories

(Core :: Graphics, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(5 files, 2 obsolete files)

We currently have 2 GL wrappers, one in Layers and one in WebGL. We should create a single wrapper inside thebes which can be used by all code in the tree.
Attachment #440377 - Flags: review?(vladimir)
This is mostly ok, with a few comments: - WGLLibrary should be a LibrarySymbolLoader. No point in manually calling getprocaddress for everything, especially when there are EXT/ARB versions of things. - There's a lot of incomplete stuff -- e.g. createpbuffer needs to take a lot of context config info, but I guess that can be fleshed out afterwards. this is probably fine to check in though, and we can expand it as needed.
Attached patch Part 2: Make Layers use new Unified OpenGL (obsolete) (deleted) — Splinter Review
This will make layers use the new Unified OpenGL architecture.
Attachment #441211 - Flags: review?(vladimir)
Updated according to IRC discussion.
Attachment #441211 - Attachment is obsolete: true
Attachment #441621 - Flags: review?(vladimir)
Attachment #441211 - Flags: review?(vladimir)
The pixel buffer options will almost definitely need adjusting. Just setting the accelerated flags appears to work for now.
Attachment #441707 - Flags: review?(vladimir)
Bas checked in these patches. Lets do followup work in new bugs.
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
FTR I checked in a couple of bustage fixes for non-libxul builds, because the patch added publicly exported functions but didn't have THEBES_API on them. http://hg.mozilla.org/mozilla-central/rev/0723bab9f15d http://hg.mozilla.org/mozilla-central/rev/29a6a85fab8e
Static Thunderbird and SeaMonkey builds on Linux are still busted from this landing, see e.g. http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1272381729.1272382167.28813.gz#err0
Oh, and the bustage on shared SeaMonkey and Thunderbird builds on Windows seems to be from this as well, see http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1272387772.1272388191.16315.gz and http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1272381159.1272386635.12140.gz for examples.
One way to fix the windows bustage as far as I can see would be to move layers into thebes.dll, but I'm not sure if that's desirable? Otherwise we'd have to export the symbols from thebes.dll. Roc or Ted have any thoughts on this?
I don't really care what you do with non-libxul builds, since we don't ship Firefox like that. We haven't unsupported it yet because Thunderbird and SeaMonkey can't build in the libxul configuration, so they can only do static or shared, and they build shared builds in order to build with tests. I'd suggest doing the right thing for libxul builds, and then doing whatever hackery you need to make shared builds work.
I think moving layers into thebes.dll would be OK if it helps.
I've been thinking over the last day or so that building layers thebes and maybe ycbcr would be best linked as one lib in shared configuration. Probably a topic for another bug: could we even go as far as all dirs in gfx as one lib?
(In reply to comment #13) > I've been thinking over the last day or so that building layers thebes and > maybe ycbcr would be best linked as one lib in shared configuration. Probably a > topic for another bug: could we even go as far as all dirs in gfx as one lib? I can live with that, gfx isn't that big, no advantage to having a seperate tiny Layers DLL as far as I can see.
I'd love to tell you non-libxul is bogus for 1.9.3, but as hard as I'm pushing for it, I only believe it once we're there. ;-) In the mean time, we have a busted Linux static and Windows shared builds on SeaMonkey and a red and basically closed tree one week before cutting an Alpha, you can imagine how uncomfortable I feel with that as a project manager... So, as long as you can fix it fast, I'm very happy with any way you can fix it (unfortunately I have no clue about C++ code myself).
So fundamentally the problem here is that Layers is re-using THEBES_API. THEBES_API is declared as dllexport or dllimport depending on whether IMPL_THEBES is defined. However THEBES_API is ment to be used for elements in the thebes module. Since we use it in layers, we cannot import anything from thebes, since we will __declspec(dllimport) it. There's two solutions as far as I can see: 1. Define LAYERS_API and IMPL_LAYERS 2. Change compile order and include layers in thebes.dll
Attached patch Include layers in thebes module (obsolete) (deleted) — Splinter Review
This should fix buildbustage on non-libxul builds my moving layers into the thebes module.
Attachment #442011 - Flags: review?(ted.mielczarek)
After clobbering and try server builds coming back it turns out previous patch had issues. The new patch changes around more and forces thebes.dll to export the dllexport interfaces from layers using gfxDllDeps.cpp.
Attachment #442011 - Attachment is obsolete: true
Attachment #442022 - Flags: review?(ted.mielczarek)
Attachment #442011 - Flags: review?(ted.mielczarek)
Comment on attachment 442022 [details] [diff] [review] Include layers in thebes module v2 [Checkin: Comment 22] >diff --git a/gfx/thebes/src/gfxDllDeps.cpp b/gfx/thebes/src/gfxDllDeps.cpp >new file mode 100644 >--- /dev/null >+++ b/gfx/thebes/src/gfxDllDeps.cpp >@@ -0,0 +1,4 @@ >+#include "Layers.h" >+#include "LayerManagerOGL.h" >+#include "BasicLayers.h" >+#include "ImageLayers.h" WARNING - MS-DOS LINE ENDINGS!
(In reply to comment #16) > 2. Change compile order and include layers in thebes.dll I'm very much for that, probably should include ycbcr as well (can go for a followup) as we needed some ugly hack there recently to make it compile everywhere in static builds.
(In reply to comment #19) > (From update of attachment 442022 [details] [diff] [review]) > >diff --git a/gfx/thebes/src/gfxDllDeps.cpp b/gfx/thebes/src/gfxDllDeps.cpp > >new file mode 100644 > >--- /dev/null > >+++ b/gfx/thebes/src/gfxDllDeps.cpp > >@@ -0,0 +1,4 @@ > >+#include "Layers.h" > >+#include "LayerManagerOGL.h" > >+#include "BasicLayers.h" > >+#include "ImageLayers.h" > WARNING - MS-DOS LINE ENDINGS! Thanks! Will fix. The patch passes try btw, so as soon as there's a green light from Ted we can land this.
Blocks: 562311
Attachment #442022 - Flags: review?(ted.mielczarek) → review+
src/thebes has dependencies on thebes, since we moved src to building before thebes. Because of the new dependencies thebes has on src. We need src/thebes to build after thebes now. We probably should restructure all this, but this should work as a quick fix to get all non-libxul builds going again.
Attachment #442237 - Flags: review?(ted.mielczarek)
Comment on attachment 442237 [details] [diff] [review] Build src/thebes after thebes [Checkin: See comment 25] -DIRS = thebes +DIRS = $(NULL) Just remove this line.
Attachment #442237 - Flags: review?(ted.mielczarek) → review+
Depends on: 562664
Depends on: 564026
Attachment #442237 - Attachment description: Build src/thebes after thebes → Build src/thebes after thebes [Checkin: See comment 25]
Attachment #442022 - Attachment description: Include layers in thebes module v2 → Include layers in thebes module v2 [Checkin: Comment 22]
Attachment #440377 - Attachment description: Part 1: Add Unified GL Wrapper → Part 1: Add Unified GL Wrapper [Checkin: Comment 6]
Attachment #441621 - Attachment description: Part 2: Make Layers use new Unified OpenGL v2 → Part 2: Make Layers use new Unified OpenGL v2 [Checkin: Comment 6]
Attachment #441707 - Attachment description: OSX backend, and relevant cocoa widget changes to enable usage → OSX backend, and relevant cocoa widget changes to enable usage [Checkin: Comment 6]
Attachment #441707 - Attachment description: OSX backend, and relevant cocoa widget changes to enable usage [Checkin: Comment 6] → OSX backend, and relevant cocoa widget changes to enable usage
Blocks: 564606
Target Milestone: --- → mozilla1.9.3a5
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: