make scroll layer items report the correct bounds and visible region

RESOLVED FIXED in Firefox 28



6 years ago
5 years ago


(Reporter: tnikkel, Assigned: tnikkel)


27 Branch
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox28+ fixed, firefox29 fixed)



(1 attachment)



6 years ago
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.

Comment 1

6 years ago
Posted patch patchSplinter Review
Attachment #8349125 - Flags: review?(roc)
Closed: 6 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.
Duplicate of this bug: 932055
This should track 28 since bug 932055 was tracking 28.
Duplicate of this bug: 941003
Let's get an uplift nomination on this.

Comment 9

6 years ago
Comment on attachment 8349125 [details] [diff] [review]

[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?

Comment 12

6 years ago
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)


5 years ago
Depends on: 966510
You need to log in before you can comment on or make changes to this bug.