996839 - Improve on GLContextProvider implementation

Status: NEW

(Core :: Graphics, enhancement)

Description

[Kelsey Gilbert [:jgilbert]]

I think that GLContextProvider is messy and hacky. Maybe we can be less hacky about it?

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: straw man 1

Comment 2

[Andreas Gal :gal]

Thats a lot of changes. If any of them breaks anything, you will be backing out a huge patch. Why don't you submit small individual fixes? And also, is this the highest priority work to do right now?

Comment 3

[Kelsey Gilbert [:jgilbert]]

Ugh, evidently part of the huge size is files with whitespace at EOL, which my editor strips.
The core of these changes aren't that big.

This isn't high priority to fix, but I don't know if it will ever be.
We could suffer along with our current version, but it means adding more #ifdefs throughout client code, since GLContextProviderEGL, for example, only exists on platforms that can build it.

I don't think it should take much time to clean this up a bit, so why not talk about it now? :)

This patch isn't really possible to split up and maintain buildability. I could split it anyways, but I think that this is pretty straight-forward to review. It only has two main parts, the GLContextProvider refactor, and the repair of calling code-sites.

Comment 4

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

Comment on attachment 8407108 [details] [diff] [review]
straw man 1

So if we want to have this discussion, what we need is a whiteboard, not patches at this point.

It might totally be the case that this patch is what we'll want to do, but at this point I would rather spend an hour chatting about what we want, than spend an hour reading code for a particular implementation of a particular possible approach.