Some helpers and bug fixes for picture caching work.

[gw3583]

From the individual commits:

• Fix debug assert due to color being out of bounds.
• Store whether a scroll frame is implicitly added or not.
• Add a fast path for no-op stacking contexts.
• Fix texture cache eviction of tiles that are still useful.

---

This change is 

[gw3583]

r? @kvark or @nical

Pending try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d4c1c6f24f1f068b770047c7037a00ee83c0817

[gw3583]

Try run looks good.

[kvark]

Reviewed 7 of 7 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @gw3583)

---

webrender/src/clip_scroll_tree.rs, line 473 at r1 (raw file):

                SpatialNodeType::ScrollFrame(ref info) => {
                    // Assume not identity since it may change with scrolling.
                    if !info.is_pipeline_root {

isn't the root scroll frame also... scrollable? (hence, non-identity)

---

webrender/src/display_list_flattener.rs, line 1368 at r1 (raw file):

            content_size,
            ScrollSensitivity::ScriptAndInputEvents,
            true,

it would look much cleaner at call sites if this was an enum

---

webrender/src/display_list_flattener.rs, line 2253 at r1 (raw file):

        }

        // It is redundant!

do you have any idea on how often this happens in the wild? just curious :)

---

webrender/src/util.rs, line 591 at r1 (raw file):

                offset == TypedVector2D::zero()
            }
            FastTransform::Transform { transform, .. } => {

I'm a bit worried that this would semantically cause the copy of transform, where we should be fine just comparing a reference to it

[gw3583]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @kvark)

---

webrender/src/clip_scroll_tree.rs, line 473 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

isn't the root scroll frame also... scrollable? (hence, non-identity)

It doesn't seem so - there is an implicit scroll frame being added by WR, but it doesn't seem to be used. As a follow up, I'd like to investigate removing it - I discussed this briefly with kats.

---

webrender/src/display_list_flattener.rs, line 1368 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

it would look much cleaner at call sites if this was an enum

Fixed

---

webrender/src/display_list_flattener.rs, line 2253 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

do you have any idea on how often this happens in the wild? just curious :)

It happens quite often :) Gecko provides stacking contexts for groupings in quite a lot of scenarios, and definitely always at least one or two in the main scene. Handling this case for the main scene simplifies the case of picking scroll roots to act as picture caching layers.

---

webrender/src/util.rs, line 591 at r1 (raw file):

Previously, kvark (Dzmitry Malyshau) wrote…

I'm a bit worried that this would semantically cause the copy of transform, where we should be fine just comparing a reference to it

Fixed (if what you meant is to use ref here)

[gw3583]

@kvark Thanks, all comments addressed / fixed, I think.

[kvark]

Thanks!
@bors-servo r+

[bors-servo]

📌 Commit 1bb583d has been approved by kvark

[bors-servo]

⌛ Testing commit 1bb583d with merge d771bae...

[bors-servo]

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing d771bae to master...