912042 - Avoid including GLContext.h in headers that don't need it

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

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

Attachment: remove-useless-GLContext-h-includes

GLContext.h is a big header, itself including many headers.

It's worth avoiding #including it where we don't actually need it, especially in other headers.

After this patch, here are the only remaining headers #including GLContext.h:

  $ find . -type f -name '*.h' | xargs egrep '\#include.*[^a-zA-Z0-9_]GLContext\.h' | cut -d ':' -f 1 | sort | uniq | sed 's|\.\/||g'

  gfx/layers/CopyableCanvasLayer.h
  gfx/layers/d3d9/CanvasLayerD3D9.h
  gfx/layers/SharedTextureImage.h

These 3 headers are included only once or a few times each.

Now, touching GLContext.h and rebuilding Gecko only rebuilds 58 object files, 23 of which are WebGL.

Speaking of WebGL, it currently has 45 object files; prior to this patch, they were all including GLContext.h, now only 23 of them are (as just mentioned above).

A few changes were made to and around GLContext.h along the way:

* ExtensionBitSet had a comment explaining it should be a std::bitset but that used to give compile errors on Mac, referencing an old bug. Hopefully that's resolved now. Killed ExtensionBitSet, now using std::bitset, and the function ExtensionBitSet::Load is now a static GLContext method, InitializeExtensionsBitSet.

* GLContext contained a few nested enum types. Nested enum types are impossible (AFAIK) to forward-declare. Moved them out of GLContext, still in namespace gl.

* These enums were moved to GLContextTypes.h which also gained a forward-decl of class GLContext, making it 'the right header to include' for most people.

* Of course, lots of code was moved from inline functions in headers into cpp files. A couple cpp files were created as some classes were entirely defined in headers.

Try push:
https://tbpl.mozilla.org/?tree=Try&rev=fbe08467d685

Comment 1

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

Errors caused by -Werror and double #defines, not worth updating the patch for review, but updated try push:
https://tbpl.mozilla.org/?tree=Try&rev=d2b6c836ec23

Comment 2

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

As long as there are tryservers, there is hope.
https://tbpl.mozilla.org/?tree=Try&rev=1d5aba99f1f6

Comment 3

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

https://tbpl.mozilla.org/?tree=Try&rev=2444ba914d17

Comment 4

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

New push for Win and Mac, which were still failing:
https://tbpl.mozilla.org/?tree=Try&rev=dd7c30c8fad3

Comment 5

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

Hopefully last try run, with some tests:
https://tbpl.mozilla.org/?tree=Try&rev=30661fa2a517

Comment 6

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

Another iteration on Mac only:
https://tbpl.mozilla.org/?tree=Try&rev=79ecd460ccd1

Comment 7

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

Yet another on Mac only:
https://tbpl.mozilla.org/?tree=Try&rev=b9333e675582

Comment 8

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

Green at last! landing!

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c04fad82d85

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5c04fad82d85

Comment 11

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

Attachment: Remove the last occurences of #include "GLContext.h" in remaining headers

There is, after all, no reason to stop half-way! There remained a few headers including GLContext.h, and on second look fixing it was easy too.

Now we are able to give people a very simple message: do not include GLContext.h in headers; instead, include GLContextTypes.h.

On try:
https://tbpl.mozilla.org/?tree=Try&rev=7ec397814580

Comment 12

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

Forgot to include GLContext.h in CanvasLayerD3D9.cpp, new try:
https://tbpl.mozilla.org/?tree=Try&rev=3b7aa76f1778

Comment 13

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

http://hg.mozilla.org/integration/mozilla-inbound/rev/59d8b8c4f9f6

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/59d8b8c4f9f6