Closed Bug 951467 Opened 10 years ago Closed 10 years ago

make scroll layer items report the correct bounds and visible region

Categories

(Core :: Layout, defect)

27 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla29
Tracking Status
firefox28 + fixed
firefox29 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

Currently the bounds of scroll layer items are just the union of the bounds of child items (inherited from nsDisplayWrapList). This has two problems. If a displayport is set the bounds will be much larger than what is drawn to the screen (only part of the displayport will be composited and clipped into the scroll port). Also the displayport can legally be set to be something that does not intersect the scroll port at all (if we are panning quickly), making the (clipped) bounds empty. This was (one of) the issues in bug 936500. We fixed it by making scroll layer items always visible even if they have empty visible rect.

Instead what we really need is a concept of 'inside' bounds/visible region and 'outside' bounds visible region, with scroll layer items handling the any translation needed and giving the correct bounds wherever they are needed. The outside bounds of scroll layer items are just the scroll port, the inside bounds are the display port. This is similar to pre-rendered transform items that have an expanding visible region for things that aren't (yet) visible.
Attached patch patchSplinter Review
Attachment #8349125 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/21837cd4fcb1
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Thank you! This fixes at least bug 932055 on Metro, and there are other bugs that may turn out to be duplicates as well.
This should track 28 since bug 932055 was tracking 28.
Let's get an uplift nomination on this.
Comment on attachment 8349125 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): not sure, the code has been this way for a while, it might have just been exposed more recently due to other changes.
User impact if declined: fixes at least bug 932055 and bug 941003
Testing completed (on m-c, etc.): on m-c for a few weeks now
Risk to taking this patch (and alternatives if risky): changes should be well contained, regressions should show up relatively quickly
String or IDL/UUID changes made by this patch: none
Attachment #8349125 - Flags: approval-mozilla-aurora?
Attachment #8349125 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Does this have or need tests?
Flags: needinfo?(tnikkel)
Flags: in-testsuite?
It could use some sure, but it's not immediately clear to me how to write one as I never reproduced bug 932055 or bug 941003.
Flags: needinfo?(tnikkel)
Depends on: 966510
You need to log in before you can comment on or make changes to this bug.