560147 - Create unified GL Wrapper

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Bas Schouten (:bas.schouten)]

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.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Add Unified GL Wrapper [Checkin: Comment 6]

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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.

Comment 3

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Make Layers use new Unified OpenGL

This will make layers use the new Unified OpenGL architecture.

Comment 4

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Make Layers use new Unified OpenGL v2 [Checkin: Comment 6]

Updated according to IRC discussion.

Comment 5

[Matt Woodrow (:mattwoodrow)]

Attachment: OSX backend, and relevant cocoa widget changes to enable usage

The pixel buffer options will almost definitely need adjusting. Just setting the accelerated flags appears to work for now.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Bas checked in these patches. Lets do followup work in new bugs.

Comment 7

[Mark Banner (:standard8)]

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

[Robert Kaiser]

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

[Robert Kaiser]

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.

Comment 10

[Bas Schouten (:bas.schouten)]

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

[(not currently active) Ted Mielczarek]

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.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think moving layers into thebes.dll would be OK if it helps.

Comment 13

[Mark Banner (:standard8)]

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?

Comment 14

[Bas Schouten (:bas.schouten)]

(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

[Robert Kaiser]

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).

Comment 16

[Bas Schouten (:bas.schouten)]

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

Comment 17

[Bas Schouten (:bas.schouten)]

Attachment: Include layers in thebes module

This should fix buildbustage on non-libxul builds my moving layers into the thebes module.

Comment 18

[Bas Schouten (:bas.schouten)]

Attachment: Include layers in thebes module v2 [Checkin: Comment 22]

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.

Comment 19

[[e-mail]]

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

[Robert Kaiser]

(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.

Comment 21

[Bas Schouten (:bas.schouten)]

(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.

Comment 22

[Bas Schouten (:bas.schouten)]

Pushed as per http://hg.mozilla.org/mozilla-central/rev/5d011d796c84.

Comment 23

[Bas Schouten (:bas.schouten)]

Attachment: Build src/thebes after thebes [Checkin: See comment 25]

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.

Comment 24

[(not currently active) 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.

Comment 25

[Bas Schouten (:bas.schouten)]

Pushed http://hg.mozilla.org/mozilla-central/rev/74e96463cdf9.