Remove the gfxContext(gfxASurface*) constructor and lots of code that's then dead as a result

RESOLVED FIXED in mozilla33

Status

()

defect
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

(Blocks 1 bug)

Trunk
mozilla33
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(11 attachments)

2.08 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
1.30 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
1.38 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
10.95 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
6.10 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
1.38 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
13.50 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
1.69 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
7.88 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
2.07 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
63.35 KB, patch
bas.schouten
: review+
Details | Diff | Splinter Review
Until we completely get rid of Thebes backed gfxContexts we're going to need to maintain Thebes and Moz2D code paths in lots of places in the tree. Matt has started getting rid of these in bug 922942, but there are still 70+ locations where the gfxContext(gfxASurface*) constructor is being invoked in the tree:

https://mxr.mozilla.org/mozilla-central/search?string=gfxContext%28&filter=[sS]urface&tree=mozilla-central
Depends on: 932198
Interested in helping here. I will work that list backwards.
Depends on: 933030
Depends on: 948221
Depends on: 965967
Depends on: 988793
Depends on: 934183
Depends on: 988808
Depends on: 988818
Depends on: 988820
Depends on: 988824
Depends on: 991572
Depends on: 991575
Depends on: 991640
Depends on: 998113
Depends on: 1001687
Depends on: 1018187
Depends on: 1018196
Depends on: 987194
Depends on: 1019018
Depends on: 1019492
Depends on: 1019681
Depends on: 1019690
Depends on: 1019704
Depends on: 1021973
Depends on: 1022031
Depends on: 1023522
Depends on: 1025802
Depends on: 1028786
Depends on: 1028794
Depends on: 1028825
Depends on: 1029211
Depends on: 1029455
Depends on: 1031055
Depends on: 1031079
Depends on: 1033607
Depends on: 1034399
Depends on: 1034404
Depends on: 1034528
Assignee: nobody → jwatt
Keywords: leave-open
Summary: Eliminate all Thebes backed gfxContexts (i.e. consumers of the gfxContext(gfxASurface*) constructor) → Remove the gfxContext(gfxASurface*) constructor and lots of code that's then dead as a result
Attachment #8451048 - Attachment description: part 3 - Remove the code for handling Thebes backed gfxContext in gfxDrawable.cpp → part 1 - Remove the gfxContext(gfxASurface*) constructor
Attachment #8451048 - Flags: review?(bas) → review+
Attachment #8451049 - Flags: review?(bas) → review+
Attachment #8451050 - Flags: review?(bas) → review+
Attachment #8451054 - Flags: review?(bas) → review+
Attachment #8451056 - Flags: review?(bas) → review+
Attachment #8451057 - Flags: review?(bas) → review+
Comment on attachment 8451059 [details] [diff] [review]
part 7 - Remove code for handling Thebes backed gfxContext from various files under layout/base/

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

::: layout/base/nsCSSRendering.cpp
@@ +2542,1 @@
>    }

nit: I suspect this brace needs to go...
Attachment #8451059 - Flags: review?(bas) → review+
Attachment #8451063 - Flags: review?(bas) → review+
Attachment #8451068 - Flags: review?(bas) → review+
Attachment #8451069 - Flags: review?(bas) → review+
Comment on attachment 8451089 [details] [diff] [review]
part 11 - Remove gfxContext::mCairo

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

W00t!!!!

::: gfx/thebes/gfxContext.cpp
@@ -162,5 @@
> -
> -    if (dx && dy)
> -        cairo_surface_get_device_offset(s, dx, dy);
> -    return gfxASurface::Wrap(s);
> -  } else {

nit: This ought to need a closing brace removed somewhere... and some indentation change.
Attachment #8451089 - Flags: review?(bas) → review+
Somehow the removal of CopySurface ended up in a later (not yet pushed) patch in my queue. Pushed separately:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e13ecd9e3def
No longer depends on: 934183
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Thanks. :)
You need to log in before you can comment on or make changes to this bug.