gfx/2D.h is a huge mess. :( It's a monolithic header which pulls in a ton of mostly unneeded header into lots of code across the tree. Here's an example: nsLayoutUtils.h wants gfxPattern::GraphicsFilter, so it #includes gfxPattern.h. That pulls in gfx/2D.h. Then everything that this header adds in is #included across the tree. Can we break up 2D.h into a one header per class model, to minimize the dependency hell here?

I had a chat with Bas on irc yesterday. 2D.h is not really a huge mess. It is fairly rare to need anything in 2D.h without most of the rest of it. Even if we split it up it is unlikely we could reduce the headers required for any of the resulting headers. The includes for 2D.h are: #include "Types.h" #include "Point.h" #include "Rect.h" #include "Matrix.h" #include "UserData.h" #include "mozilla/GenericRefCounted.h" #include "mozilla/RefPtr.h" Post pre-processing, 2D.h is pretty huge, apx 1.9Mb, but (surprisingly) the bulk of that is due to Rect.h (1.2Mb) - which pulls in algorithm and cmath. If we were to split 2D.h, then we would probably still need Rect in all the new smaller headers. Unfortunately we can't easily split BaseRect.h (which is included in Rect.h) into a cpp and header because it is all templates. The optimum solution is probably to implement max, min, floor, and ceil ourselves rather than pull in the std headers.

(In reply to Nick Cameron [:nrc] from comment #1) > Post pre-processing, 2D.h is pretty huge, apx 1.9Mb, but (surprisingly) the > bulk of that is due to Rect.h (1.2Mb) - which pulls in algorithm and cmath. > If we were to split 2D.h, then we would probably still need Rect in all the > new smaller headers. Unfortunately we can't easily split BaseRect.h (which > is included in Rect.h) into a cpp and header because it is all templates. This is bad, especially since BaseRect (through nsRect/gfxRect) is in itself pulled in throughout the tree as well. It might not be impossible to put stuff into a .cpp though if we really want to, we simply need to have functions like: foo(T) declared for each T we actually use BaseRect with, and then define them in a .cpp file, we then simply call them from the templated functions and the template will fail to compile for a T they're not defined for. Alternatively we could just do the stuff we do through algorithm and cmath right now by hand. (In reply to :Ehsan Akhgari (needinfo? me!) from comment #0) > gfx/2D.h is a huge mess. :( It's a monolithic header which pulls in a ton > of mostly unneeded header into lots of code across the tree. Here's an > example: In the future in situations like this please include why it is a 'mess', this is a rather loaded statement and when making it stronger arguments should be included. At least one statement in here is objectively untrue - the list of headers Nick uses are types which are all but one (very small header - UserData.h) very common in graphics code. And more importantly all of them except Matrix are used by almost every class in 2D.h.

(In reply to Bas Schouten (:bas.schouten) from comment #2) > (In reply to :Ehsan Akhgari (needinfo? me!) from comment #0) > > gfx/2D.h is a huge mess. :( It's a monolithic header which pulls in a ton > > of mostly unneeded header into lots of code across the tree. Here's an > > example: > > In the future in situations like this please include why it is a 'mess', > this is a rather loaded statement and when making it stronger arguments > should be included. At least one statement in here is objectively untrue - > the list of headers Nick uses are types which are all but one (very small > header - UserData.h) very common in graphics code. And more importantly all > of them except Matrix are used by almost every class in 2D.h. First of all, I apologize for my tone, I was very frustrated when I filed this bug. I did not mean to criticize you or anybody else personally. I actually gave the reason why this is a problem in comment 0, the 2nd paragraph. nsLayoutUtils.h is commonly included in layout/ and given what I wrote in that comment, touching 2D.h pretty much means all of layout needs to be recompiled. So, I'm not only talking about recompiling gfx/, the situation is worse than that. The other place in our code base where we have a similar kind of header is the JS engine (jsapi.h, etc.) and the JS folks have been working hard at minimizing the number of places that header needs to be (transitively) included. It would be awesome if we did the same thing about 2D.h. I'd do that work myself, but when I discussed this with Jeff he told me that 2D.h is intentionally this way (it is supposed to be the only header one should include to access any of the gfx/2d APIs), and also splitting it up by someone who doesn't know the code is very hard since there are tons of inline functions there and it's not immediately obvious which ones are actually important for performance and which ones the PGO compiler can inline for us. Thanks!

I wonder if we can fix the uses of 2D.h - gfxPattern in Ehsan's example does not look like it needs to pull in 2D.h at all (it could forward declare SourceSurface). 2D.h actually gets changed fairly rarely (and is kind of small), so I'm not sure that it is so bad that it gets included in a lot of places.

(In reply to comment #4) > I wonder if we can fix the uses of 2D.h - gfxPattern in Ehsan's example does > not look like it needs to pull in 2D.h at all (it could forward declare > SourceSurface). That's not very easy since gfxPattern requires the sizes of things declared in 2D.h: <http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxPattern.h#127>

WIP patch for removing the std library headers from being removed everywhere. Obviously it needs some tidying up. But after this dev.platform discussion (https://groups.google.com/forum/#!topic/mozilla.dev.platform/qnTYibbtIrg) I don't think we should bother. But just in case at some point we want to, I thought I would leave this patch here. It does not address the wider issues around splitting up 2D.h (or not).