1205129 - Move ContainerLayer construction out of FrameLayerBuilder

Status: RESOLVED INVALID

(Core :: Layout, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The creation and destruction of ContainerLayers is currently managed by FrameLayerBuilder, which reevaluates those decisions on every paint. We should be able to avoid doing this. That would let us simplify FrameLayerBuilder.

To create a ContainerLayer, we need to know whether it's active or not, so that also needs to be pulled out of FrameLayerBuilder.

I suggest we add a frame property that holds the frame's ContainerLayers (there can be more than one ContainerLayer for a frame if it has multiple effects applying to it), plus a frame state bit to indicate the presence of the frame property, plus a frame state bit to indicate whether any of the frame's descendants have the property. This property would also manage any data that we need to track the layers' activity status.

During display list construction, we can create any necessary ContainerLayers and set their properties using ChooseScaleAndSetTransform (which would need to be migrated to the display list code). During FrameLayerBuilder, when we need to build a layer for a frame that has a ContainerLayer, we'd just pull the ContainerLayer out of the frame property and reparent it to the correct container. We'd invoke FrameLayerBuilder separately for each ContainerLayer.

During display list construction we'd need to identify frame subtrees that we skipped during frame tree traversal (because they're outside the displayport), and for any which have descendants with the ContainerLayer frame property, we'd find those descendants and delete their layers.

This would then let us implement an optimization where we track on a per-ContainerLayer basis whether we need to run FrameLayerBuilder (and potentially, display list construction) for that ContainerLayer.

Comment 1

[Matt Woodrow (:mattwoodrow)]

Ting-Yu, are you interested in working on this?

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

This looks like an interesting task to hack layer system to me. However, I'm still hunting AcessibleCaret bugs. I'm afraid I cannot commit time to do it currently. Does this block other things we want to do?

Comment 3

[Matt Woodrow (:mattwoodrow)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #0) 
> I suggest we add a frame property that holds the frame's ContainerLayers
> (there can be more than one ContainerLayer for a frame if it has multiple
> effects applying to it), plus a frame state bit to indicate the presence of
> the frame property, plus a frame state bit to indicate whether any of the
> frame's descendants have the property. This property would also manage any
> data that we need to track the layers' activity status.

We're running quite low on state bits (3 free by my count), and I want to use (at least) one for display list optimization. Do we have any plans on what to do when we run out?

> 
> During display list construction, we can create any necessary
> ContainerLayers and set their properties using ChooseScaleAndSetTransform
> (which would need to be migrated to the display list code). During
> FrameLayerBuilder, when we need to build a layer for a frame that has a
> ContainerLayer, we'd just pull the ContainerLayer out of the frame property
> and reparent it to the correct container. We'd invoke FrameLayerBuilder
> separately for each ContainerLayer.

So, when we encounter a ContainerLayer building item during display item construction, we'd append the Container/DisplayList to a set owned by the nsDisplayListBuilder and then call BuildContainerLayerFor on each item in the set during layer building?

Comment 4

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: part 1, create LayerManagers before display items

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I count 4 bits free (35, 46, 55, 56). Just use them! If we run out, we'll probably allocate 32 more, though before we do that we should have a good look at the list and see if there are any that we no longer needed or are rarely enough accessed they can be a frame property.

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> So, when we encounter a ContainerLayer building item during display item
> construction, we'd append the Container/DisplayList to a set owned by the
> nsDisplayListBuilder and then call BuildContainerLayerFor on each item in
> the set during layer building?

FrameLayerBuilder::BuildContainerLayerFor should probably change to something like BuildChildLayers, since it won't actually (re)construct the container layer anymore. The item itself will do that, then BuildChildLayers will run.

Creating a list of all items with ContainerLayers, ordered so that parents are always later than their children in the list, and then making layer building be calling BuildChildLayers for each element of the list in order, sounds good. Although it might be possible to run BuildChildLayers for a ContainerLayer item immediately after we've finished building all the children of the item --- that might be slightly simpler, and might win on memory locality, but someone would need to check to make sure we're not doing display list surgery after the item has finished building its child items.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:dholbert, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 7

[Daniel Holbert [:dholbert]]

This is about simplifying FrameLayerBuilder, which no longer exists in our source code (except in comments). Probably removed as part of bug 1724935.

--> calling this INVALID at this point