make scroll layer items report the correct bounds and visible region

RESOLVED FIXED in Firefox 28

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: tnikkel, Assigned: tnikkel)

Tracking

27 Branch
mozilla29
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox28+ fixed, firefox29 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

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.
Assignee

Comment 1

6 years ago
Posted patch patchSplinter Review
Attachment #8349125 - Flags: review?(roc)
https://hg.mozilla.org/mozilla-central/rev/21837cd4fcb1
Status: NEW → RESOLVED
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.
Assignee

Comment 9

6 years ago
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?
Assignee

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)
Assignee

Updated

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