Closed
Bug 1296828
Opened 8 years ago
Closed 8 years ago
Store ImageKeys and SurfaceKeys directly on ISurfaceProviders
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: seth, Assigned: seth)
References
Details
Attachments
(3 files)
1.46 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
16.01 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
(Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders.
19.75 KB,
patch
|
dholbert
:
review+
|
Details | Diff | Splinter Review |
SurfaceCache cache entries are CachedSurface objects that wrap ISurfaceProviders, and it happens that ISurfaceProviders generally need to know their own ImageKey and SurfaceKey. However, CachedSurface redundantly stores this information itself. It makes sense to just get the ImageKey and SurfaceKey from the ISurfaceProvider, which will both use less memory and ensure that the two sets of values can't get out of sync.
Assignee | ||
Comment 1•8 years ago
|
||
This patch just fixes a couple of places where we were wrongly using Image* raw pointers instead of ImageKey values. It'd probably be smart to prevent that using the type system, but that's better left for a different bug.
Attachment #8784586 -
Flags: review?(dholbert)
Assignee | ||
Comment 2•8 years ago
|
||
This patch makes ImageKeys and SurfaceKeys accessible via the ISurfaceProvider interface. Note that things get a bit more verbose here because ISurfaceProvider and the SurfaceCache methods are both getting passed the same values, but that'll go away in the next part.
Attachment #8784587 -
Flags: review?(edwin)
Attachment #8784587 -
Flags: review?(dholbert)
Assignee | ||
Comment 3•8 years ago
|
||
Now that part 2 is done, we can start relying on the ISurfaceProvider to know its own ImageKey and SurfaceKey within the SurfaceCache code. We're gradually whittling away the need for CachedSurface to exist at all!
Attachment #8784588 -
Flags: review?(dholbert)
Assignee | ||
Comment 4•8 years ago
|
||
Try job here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dfa5a7237ff3
Comment 5•8 years ago
|
||
Comment on attachment 8784586 [details] [diff] [review] (Part 1) - Use ImageKey consistently in SurfaceCache. Review of attachment 8784586 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8784586 -
Flags: review?(dholbert) → review+
Comment 6•8 years ago
|
||
Comment on attachment 8784588 [details] [diff] [review] (Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders. Review of attachment 8784588 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8784588 -
Flags: review?(dholbert) → review+
Comment 7•8 years ago
|
||
Comment on attachment 8784587 [details] [diff] [review] (Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. Review of attachment 8784587 [details] [diff] [review]: ----------------------------------------------------------------- r=me (Also: I'm not sure it's worth having edwin review this as well; I'm happy to call this whole patch r=me, unless there's something that you'd like edwin to look at in particular.)
Attachment #8784587 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 8•8 years ago
|
||
Thanks for the reviews! (In reply to Daniel Holbert [:dholbert] from comment #7) > (Also: I'm not sure it's worth having edwin review this as well; I'm happy > to call this whole patch r=me, unless there's something that you'd like > edwin to look at in particular.) Works for me!
Pushed by seth.bugzilla@blackhail.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/39e0894d2dfe (Part 1) - Use ImageKey consistently in SurfaceCache. r=dholbert https://hg.mozilla.org/integration/mozilla-inbound/rev/8f8d3c106410 (Part 2) - Store ImageKeys and SurfaceKeys directly on ISurfaceProviders. r=dholbert,edwin https://hg.mozilla.org/integration/mozilla-inbound/rev/d28b09df98ff (Part 3) - Update SurfaceCache API to rely on ImageKeys and SurfaceKeys stored on ISurfaceProviders. r=dholbert
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/39e0894d2dfe https://hg.mozilla.org/mozilla-central/rev/8f8d3c106410 https://hg.mozilla.org/mozilla-central/rev/d28b09df98ff
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Attachment #8784587 -
Flags: review?(edwin)
You need to log in
before you can comment on or make changes to this bug.
Description
•