901498 - Make GLContext more modular, with a core doing only the OpenGL entry points, and any extra features deferred to separate classes

Status: RESOLVED WONTFIX

(Core :: Graphics, task)

Description

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

From the etherpad (glcontext) :

*GLContext refactoring*

Sic transeunt kludgea mundi (Thus pass the kludges of the world)

Immediate goal: make GLContext multiplexing implementable without making GLContext more of a kludge than it already is.
Secondary goal: the same for a few other GLContext features including: compiled unit tests, remote GL calls over IPC (we may not want to ship this, but we should be able to at least toy with it), a built-in implementation of APItrace, ... generally make GLContext extensible without having to add all new code in that single, central, untestable class.

*Proposed class hierarchy*

GLContext -> GLContextRaw -> various GLContext implementations
GLContext -> GLContextWrapper -> various wrapper classes

*GLContext*

GLContext is the universal base class for anything that behaves like an OpenGL context and exposes an OpenGL API. The methods there are roughly the standard GL API. All these methods are pure virtual. The virtualness will be essential for wrappers (see below).

*GLContextRaw*

GLContextRaw becomes a plain raw OpenGL context with no bells or whistles. No faking of framebuffer 0, no special debugging features, no multiplexing. Nothing. The bells and whistles all move to the "wrappers", see below.
GLContextRaw would continue, at least for now, to be further specialized on each platform (GLContextEGL etc).
A special specialization of GLContextRaw would allow writing compiled-code unit tests that cover the internal logic in our wrappers (see below) and don't do actual GL calls. Maybe that'd be just a "null" GLContext type, with any logging or other testing features being provided by a wrapper.

*GLContextWrapper*

GLContextWrapper is, besides GLContextRaw, the other direct subclass of GLContext. All what GLContextWrapper does is wrap a GLContext*, expose a GLContext API itself, and trivially forward all these GL calls to its wrapped GLContext. Note, again, that all of these forwarding functions are virtual.
Where it gets interesting is that we would again subclass GLContextWrapper to implement special wrappers offering one feature each. Having the trivial GLContextWrapper class to inherit means that each wrapper only has to implement the functions that it wants to affect. For many of the wrappers, that will considerably reduce the amount of boilerplate code.
Wrappers to be implemented:

 1. Multiplexing (implementing virtual GLContext-like things that share a single actual OpenGL context)

 2. WorkAroundDriverBugs (move this pile of ugly code to a single separate place instead of polluting all of GLContext).

 3. MOZ_GL_DEBUG (move this debugging helper to a separate place instead of polluting all of GLContext)

 4. APItrace (implement our own APItrace-compatible logging so it's easy to use everywhere, which APItrace isn't)

 5. Offscreen (move all the custom ScreenBuffer / framebuffer-zero code to a separate wrapper class instead of complicating all of GLContext --- even after the introduction of GLScreenBuffer there is still lots of offscreen-specific stuff in GLContext)

 6. Remote-GL-over-IPC (we may not actually want to ship this, but it'd be nice to be able to toy with it easily. A wrapper could handle this).

 7. Shadowing of GL state (store e.g. object bindings in our own data structures, useful for many things e.g. memory reporting. Again, don't want to complicate all of GLContext for that).

*Difficulties*

Some of the above wrappers, namely Offscreen and Shadowing, need to expose custom API in addition to the standard GL API. It'd be ugly to have to add these extra calls to GLContext, and that'd be slow too (the chain of virtual calls has an overhead that may be more significant there as these extra calls may be less inherently expensive than regular GL calls).
As long as the extra calls are read-only accessors, we can allow calling these methods directly on the wrapped class, so this is a non-issue.
That seems to cover the Shadowing case well. It's not clear to me if this covers Offscreen well. If it doesn't, then we can get away by making Offscreen special in that it can't be wrapped --- so that if you want to combine Offscreen with other wrappers, then Offscreen must be the outmost wrappers.
In the present design, each GL call goes through a chain of N virtual function calls where N is the number of nested wrappers. Since only GL API calls are to be wrapped in this way, and GL calls are considered expensive anyway, this shouldn't be a problem; if this turns out to be a problem, we can switch from virtual to templates for the implementation, but let's not do that until that's shown to be needed.

Conversation on etherpad:

*jgilbert* : It should also be possible to collapse the virtual calls by handling the pfn calls ourselves, basically doing c++ monkey-patching.
*bjacob* : How is that different from virtual calls? I mean, each wrapper wants to add a level of indirection; pfn or virtual is the same, no?
*jgilbert* : If all N wrapper levels just forward the call, we should be able to just call the final level, instead of traversing the list of wrappers. Thus we make only the minimal number of indirect calls.
*bjacob* : Oh i see. How would we notice that that is the case and that we can do this optimization for those calls?
*jgilbert* : Instead of making wrapper classes with inheritance, etc., we have patching functions for each type of layer. The patching function is responsible for recording the previous pfn for a GL call, and patching it to do what it wants, calling the previous pfn as needed.
*bjacob* : Oh i see, that sounds like a good idea if performance issues happen. Though it won't help with wrappers that touch all GL functions. But looking at the list of wrappers, it's true that most of the ones we'd use in production affect only a minority of GL calls each. The one exception (production-use and affects all calls) would be remote-IPC.
*jgilbert* : True, but neither should it hurt them. The code to do it is messier than just using virtual wrappers, though.
*bjacob* : Yeah, so i'd like to only do that if any performance issue shows up.

Comment 1

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

Had a discussion on irc yesterday -- this IMO isn't high priority; the biggest issue is canvas 2d on SkiaGL needs to not use a GLContext per canvas.  That's doable now, via sharing a GrContext amongst all of the SkGpuDevices (already supported); we just need to use the right kind of texture client/host to export a specific texture id via EGLImage to the compositor from the shared canvas context.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 3

[Kelsey Gilbert [:jgilbert]]

I don't think we want to do this anymore.