Closed
Bug 560147
Opened 15 years ago
Closed 15 years ago
Create unified GL Wrapper
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.3a5
People
(Reporter: bas.schouten, Assigned: bas.schouten)
References
Details
Attachments
(5 files, 2 obsolete files)
(deleted),
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vlad
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•15 years ago
|
||
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.
Attachment #440377 -
Flags: review?(vladimir) → review+
Assignee | ||
Comment 3•15 years ago
|
||
This will make layers use the new Unified OpenGL architecture.
Attachment #441211 -
Flags: review?(vladimir)
Assignee | ||
Comment 4•15 years ago
|
||
Updated according to IRC discussion.
Attachment #441211 -
Attachment is obsolete: true
Attachment #441621 -
Flags: review?(vladimir)
Attachment #441211 -
Flags: review?(vladimir)
Attachment #441621 -
Flags: review?(vladimir) → review+
Comment 5•15 years ago
|
||
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
Attachment #441707 -
Flags: review?(vladimir)
Comment 7•15 years ago
|
||
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
Comment 8•15 years ago
|
||
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
Comment 9•15 years ago
|
||
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.
Assignee | ||
Comment 10•15 years ago
|
||
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?
Comment 11•15 years ago
|
||
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.
Comment 13•15 years ago
|
||
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?
Assignee | ||
Comment 14•15 years ago
|
||
(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.
Comment 15•15 years ago
|
||
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).
Assignee | ||
Comment 16•15 years ago
|
||
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
Assignee | ||
Comment 17•15 years ago
|
||
This should fix buildbustage on non-libxul builds my moving layers into the thebes module.
Attachment #442011 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 18•15 years ago
|
||
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 19•15 years ago
|
||
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!
Comment 20•15 years ago
|
||
(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.
Assignee | ||
Comment 21•15 years ago
|
||
(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.
Updated•15 years ago
|
Attachment #442022 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 22•15 years ago
|
||
Pushed as per http://hg.mozilla.org/mozilla-central/rev/5d011d796c84.
Assignee | ||
Comment 23•15 years ago
|
||
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 24•15 years ago
|
||
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+
Assignee | ||
Comment 25•15 years ago
|
||
Updated•15 years ago
|
Attachment #442237 -
Attachment description: Build src/thebes after thebes → Build src/thebes after thebes
[Checkin: See comment 25]
Updated•15 years ago
|
Attachment #442022 -
Attachment description: Include layers in thebes module v2 → Include layers in thebes module v2
[Checkin: Comment 22]
Updated•15 years ago
|
Attachment #440377 -
Attachment description: Part 1: Add Unified GL Wrapper → Part 1: Add Unified GL Wrapper
[Checkin: Comment 6]
Updated•15 years ago
|
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]
Updated•15 years ago
|
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]
Updated•15 years ago
|
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
You need to log in
before you can comment on or make changes to this bug.
Description
•