Closed
Bug 1000266
Opened 10 years ago
Closed 10 years ago
don't purge the canvas background image cache all the time, making it useless
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: tnikkel, Assigned: tnikkel)
References
Details
Attachments
(1 file)
7.60 KB,
patch
|
mattwoodrow
:
review+
|
Details | Diff | Splinter Review |
Bug 818643 was a problem where we didn't purge the cache enough. The fix was to basically always purge the cache. Making it useless. We should either fix this or get rid of the cache, I couldn't tell exactly what led to it's creation based on the code history/bug. This is super hacky.
Attachment #8411077 -
Flags: review?(matt.woodrow)
Assignee | ||
Comment 1•10 years ago
|
||
This does have a measurable perf impact on the cases it affects though (otherwise there'd be no point in all the hackyness!).
Comment 2•10 years ago
|
||
This was originally added because we don't have component alpha layers with BasicLayers. Instead we have to flatten layers together as best we can to try keep subpixel AA enabled for text. Unfortunately this means we have to repaint the whole page on scroll if we have text on top of a fixed background. I'm surprised this regressed, since it was a big win for tscrollx when it was added.
Comment 3•10 years ago
|
||
Comment on attachment 8411077 [details] [diff] [review] patch Review of attachment 8411077 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/FrameLayerBuilder.cpp @@ +2687,5 @@ > } else { > // Let the display item check for geometry changes and decide what needs to be > // repainted. > + > + if (oldGeometry->ComputeInvalidationRegion() == aGeometry->ComputeInvalidationRegion() && A comment (similar to the one in the commit message) would be really good here.
Attachment #8411077 -
Flags: review?(matt.woodrow) → review+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Matt Woodrow (:mattwoodrow) from comment #2) > This was originally added because we don't have component alpha layers with > BasicLayers. Instead we have to flatten layers together as best we can to > try keep subpixel AA enabled for text. > > Unfortunately this means we have to repaint the whole page on scroll if we > have text on top of a fixed background. That's what I figured. > I'm surprised this regressed, since it was a big win for tscrollx when it > was added. This patch gets about 4% on linux tscrollx, and I'm guessing that's the only place we could see a difference because everywhere else is using non-basic layers. Maybe tscroll has changed since then. Strangely putting a return at the start of the canvas background image paint function gets less of a win than that. Which I cannot explain.
Assignee | ||
Comment 5•10 years ago
|
||
Added a huge paragraph of a comment: https://hg.mozilla.org/integration/mozilla-inbound/rev/671620466491
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/671620466491
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•