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

RESOLVED FIXED in mozilla27

Status

()

Core
Graphics
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Away for a while, Assigned: Away for a while)

Tracking

(Blocks: 1 bug)

Trunk
mozilla27
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
This will hopefully help minimize the impact of this header across our code base.
(Assignee)

Comment 1

4 years ago
Created attachment 811577 [details] [diff] [review]
Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h
Attachment #811577 - Flags: review?(roc)
(Assignee)

Comment 2

4 years ago
Created attachment 811578 [details] [diff] [review]
Part 2: Move GraphicsFilters outside of gfxPattern.h so that we won't need to #include that header everywhere GraphicsFilter is needed
Attachment #811578 - Flags: review?(roc)
(Assignee)

Comment 3

4 years ago
Created attachment 811579 [details] [diff] [review]
Part 3: Avoid #including nsStyleStructInlines.h in nsHTMLReflowState.h
Attachment #811579 - Flags: review?(roc)
(Assignee)

Comment 4

4 years ago
Created attachment 811580 [details] [diff] [review]
Part 4: Move DrawMode outside of gfxFont.h so that we won't need to #include that header everywhere DrawMode is needed
Attachment #811580 - Flags: review?(roc)
(Assignee)

Comment 5

4 years ago
Created attachment 811581 [details] [diff] [review]
Part 5: Remove the gfx/2D.h #include from GfxMessageUtils.h
Attachment #811581 - Flags: review?(roc)
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!
Attachment #811577 - Flags: review?(roc) → review+
(Assignee)

Comment 9

4 years ago
(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?
nsIFrameInlines works I guess.
(Assignee)

Comment 11

4 years ago
Created attachment 812609 [details] [diff] [review]
Part 1: Avoid #including nsStyleStructInlines.h in nsIFrame.h
Attachment #811577 - Attachment is obsolete: true
Attachment #812609 - Flags: review?(roc)
I think this breaks build with --disable-webgl.
Depends on: 923168
Depends on: 923170
Depends on: 923188
(In reply to Honza Bambas (:mayhemer) from comment #14)
> I think this breaks build with --disable-webgl.

Filed bug 923188.
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?
(Assignee)

Comment 17

4 years ago
Created attachment 817450 [details] [diff] [review]
Fix comment 16

Normally I'd ask for a follow-up bug to be filed, but that would be kind of rude here.  ;-)
Attachment #817450 - Flags: review?(seth)
Comment on attachment 817450 [details] [diff] [review]
Fix comment 16

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

Super duper. Thanks!
Attachment #817450 - Flags: review?(seth) → review+
You need to log in before you can comment on or make changes to this bug.