Closed
Bug 881451
Opened 12 years ago
Closed 11 years ago
ThebesLayerComposite::GetCompositionBounds() needs to be fixed up in various ways
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: kats, Assigned: cwiiis)
References
Details
(Whiteboard: [leave open])
Attachments
(1 file)
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
There are at least a couple of problems with this code:
1) The logic at the top of the loop to update the scrollableLayer variable and potentially end up using it with a different parentMetrics seems very wrong.
2) The function doesn't appear to take into account any intermediate surfaces it might be rendering to.
BenWa and I were discussing this code today and it will probably misbehave when there are multiple scrollable layers in the tree, such as the multi-apzc project is designed to allow, so this will need to be fixed up.
Reporter | ||
Comment 1•11 years ago
|
||
There also appear to be various coordinate system mismatches in this function. The most obvious one is the calculation of the "scrollOffset" variable, which takes a CSSPixel value, multiplies it by LayerPixelsPerCSSPixel to get the LayerPixel value, and then divides it by something which gets back LayoutDevicePixels (or maybe CSSPixels - I'm still trying to figure that out). It then subtracts that value from a LayerPixel "content" variable, which is wrong no matter how you slice it.
Assignee | ||
Comment 3•11 years ago
|
||
So we just don't use this code at all anymore. Will attach a patch that removes the unused bits.
Assignee | ||
Comment 4•11 years ago
|
||
This code could probably use an auditing, but if we just end up shrinking it into nothingness, that'd do too.
Comment 5•11 years ago
|
||
Comment on attachment 827560 [details] [diff] [review]
Remove unused members of TiledLayerProperties
Review of attachment 827560 [details] [diff] [review]:
-----------------------------------------------------------------
r+ but this doesn't address the bug so keep the bug open.
Attachment #827560 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/a775f0d923a6
Seems we're tracking the real problem in bug 935219 now, so I'm going to co-opt this bug to track just removing these unused properties - let me know if you think that's too messy and we can agree on something nicer :)
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to Chris Lord [:cwiiis] from comment #6)
> Seems we're tracking the real problem in bug 935219 now
Bug 935219 won't affect the code in ThebesLayerComposite. I might change what mCompositionBounds is, but I don't understand any of the code in ThebesLayerComposite so I'm not going to touch it.
Comment 8•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Reporter | ||
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [leave open]
Assignee | ||
Comment 9•11 years ago
|
||
This has been fixed in various other bugs, closing.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 10•11 years ago
|
||
A patch landed here, changing resolution to FIXED.
Resolution: WORKSFORME → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•