921753 - Reduce the number of places where we #include gfx/2d.h

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[(no longer active)]

This will hopefully help minimize the impact of this header across our code base.

Comment 1

[(no longer active)]

Attachment: Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h

Comment 2

[(no longer active)]

Attachment: Part 2: Move GraphicsFilters outside of gfxPattern.h so that we won't need to #include that header everywhere GraphicsFilter is needed

Comment 3

[(no longer active)]

Attachment: Part 3: Avoid #including nsStyleStructInlines.h in nsHTMLReflowState.h

Comment 4

[(no longer active)]

Attachment: Part 4: Move DrawMode outside of gfxFont.h so that we won't need to #include that header everywhere DrawMode is needed

Comment 5

[(no longer active)]

Attachment: Part 5: Remove the gfx/2D.h #include from GfxMessageUtils.h

Comment 6

[(no longer active)]

https://tbpl.mozilla.org/?tree=Try&rev=39b7e5487ec5

Comment 7

[(no longer active)]

https://tbpl.mozilla.org/?tree=Try&rev=467f05b6a367

The windows bustage is fixed by https://hg.mozilla.org/integration/mozilla-inbound/rev/7f0a3480a460.

Comment 8

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

Comment on attachment 811577 [details] [diff] [review]
Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h

Review of attachment 811577 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsIFrame.h
@@ +2931,5 @@
> +  uint8_t GetDisplay() const;
> +  bool IsFloating() const;
> +  bool IsPositioned() const;
> +  bool IsRelativelyPositioned() const;
> +  bool IsAbsolutelyPositioned() const;

I don't think it's wise to uninline all these!

Comment 9

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> Comment on attachment 811577 [details] [diff] [review]
> Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h
> 
> Review of attachment 811577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: layout/generic/nsIFrame.h
> @@ +2931,5 @@
> > +  uint8_t GetDisplay() const;
> > +  bool IsFloating() const;
> > +  bool IsPositioned() const;
> > +  bool IsRelativelyPositioned() const;
> > +  bool IsAbsolutelyPositioned() const;
> 
> I don't think it's wise to uninline all these!

OK, the reason I did that was to get rid of nsStyleStructInlines.h.  One option is to keep them inline, and move their implementation to a new file (nsIFrameInlines.h) which the few files which require the definitions of these methods would be able to #include.  What do you think?

Comment 10

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

nsIFrameInlines works I guess.

Comment 11

[(no longer active)]

Attachment: Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h

Comment 12

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f3016318932
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbff283df8c1
https://hg.mozilla.org/integration/mozilla-inbound/rev/236de57e3215
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae6c7a11874e
https://hg.mozilla.org/integration/mozilla-inbound/rev/42de34151ea9

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/2f3016318932
https://hg.mozilla.org/mozilla-central/rev/dbff283df8c1
https://hg.mozilla.org/mozilla-central/rev/236de57e3215
https://hg.mozilla.org/mozilla-central/rev/ae6c7a11874e
https://hg.mozilla.org/mozilla-central/rev/42de34151ea9

Comment 14

[Honza Bambas (:mayhemer)]

I think this breaks build with --disable-webgl.

Comment 15

[Daniel Holbert [:dholbert]]

(In reply to Honza Bambas (:mayhemer) from comment #14)
> I think this breaks build with --disable-webgl.

Filed bug 923188.

Comment 16

[Seth Fowler [:seth] [:s2h]]

Part 2 of this patch is wrong; it switches to forward declaring gfxPattern in gfxDrawable.h, but gfxPatternDrawable both includes an nsRefPtr<gfxPattern> as a member and implements its destructor directly in the header. This causes a compilation failure if someone includes gfxDrawable.h without including gfxPattern.h first, since the destructor of nsRefPtr<gfxPattern> needs the definition of gfxPattern to be visible.

Ehsan, could you please push a followup patch to fix this?

Comment 17

[(no longer active)]

Attachment: Fix comment 16

Normally I'd ask for a follow-up bug to be filed, but that would be kind of rude here.  ;-)

Comment 18

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 817450 [details] [diff] [review]
Fix comment 16

Review of attachment 817450 [details] [diff] [review]:
-----------------------------------------------------------------

Super duper. Thanks!

Comment 19

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c505616ed8a4

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c505616ed8a4