Layer state stack #35685
Layer state stack #35685
Conversation
flow/layers/display_list_layer.cc
Outdated
| return; | ||
| } | ||
| } | ||
| auto save = context.state_stack.save(); |
There was a problem hiding this comment.
I know this state_stack.save() will return an AutoRestore Object but if I don't know the save() implement only read this line code I will misunderstand we forget to restore.
There was a problem hiding this comment.
Would a better name help?
flow/layers/layer_tree.cc
Outdated
| } | ||
| state_stack.setBuilderDelegate(builder); | ||
| } else { | ||
| state_stack.setCanvasDelegate(frame.canvas()); |
There was a problem hiding this comment.
Here, don't we need to record the frame.view_embedder canvases?
There was a problem hiding this comment.
The canvas here is a wrapper around the builder. If we set both then the builder would get double-state-notices.
| context.state_stack.clipPath(path_, true); | ||
| auto saveLayer = context.state_stack.saveLayer( | ||
| &paint_bounds(), context.checkerboard_offscreen_layers); | ||
| context.canvas->drawColor(color_); |
There was a problem hiding this comment.
Here don't we need to set the alias ?
There was a problem hiding this comment.
The AA is in the clipPath with the "true" argument. The drawColor (was originally using a drawPaint which is silly for a solid color) doesn't process AA.
flow/layers/layer_state_stack.cc
Outdated
| builder_restore_count_ = builder->getSaveCount(); | ||
| builder_ = builder; | ||
| for (auto& state : state_stack_) { | ||
| if (!state->is_backdrop_filter()) { |
There was a problem hiding this comment.
Only when it is first pushed onto the state stack. If we update the canvas to a new overlay, we don't want to re-blur everything that is behind that overlay...(do we?)...
There was a problem hiding this comment.
Oops, but we do need to do a saveLayer so that the restores match up...
There was a problem hiding this comment.
I swiched from an "is_bdf" conditional to calling a reapply method here in the delegate swap phase. That way, only BDFEntry has a different implementation that does a saveLayer without its backdrop filter.
flow/layers/backdrop_filter_layer.cc
Outdated
|
|
||
| PaintChildren(context); | ||
| } | ||
| auto save = context.state_stack.saveWithBackdropFilter(&paint_bounds(), |
There was a problem hiding this comment.
If the parent node is OpacityLayer, here we need set the alpha of paint.
There was a problem hiding this comment.
the test case is TEST_F(BackdropFilterLayerTest, OpacityInheritance)
There was a problem hiding this comment.
Opacity inheritance is not really implemented yet - there is a skeleton of a concept for it, but it is not ready for review, just an inkling of where I'm headed. I'm still working out what is the best way to manage it with the new mechanism and what the API for layers will look like.
JsouLiang
force-pushed
the
layer-state-stack
branch
from
9a2180d to
72beac3
Compare
flar
force-pushed
the
layer-state-stack
branch
from
72beac3 to
b8b1f52
Compare
| // Return a boolean indicating whether the color filtering operation can | ||
| // be applied either before or after modulating the pixels with an opacity | ||
| // value without changing the operation. | ||
| virtual bool can_commute_with_alpha() const { return false; } |
There was a problem hiding this comment.
Here, if mostly CF can commute with alpha, but if the CF is a MtrixCF and has specific alpha in the matrix it should not commute with alpha?
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. |
|
The rebase got so complicated that I basically started over. The new version is at #36458 |
2 participants
Fundamentally, this new object records all potential state changes that
Layerobjects might impose on their children.This new object replaces a number of objects used to track state within the engine layers, some immediately and some remain to be replaced after further development:
PaintContext::internal_nodes_canvasPaintContext::builder_multiplexerinternal_nodes_canvasfor the builderPrerollContext::mutators_stackMutatorsStackobject and can eventually replace that object in thePrerollContextso that the same API is used in bothPrerollandPaintto manage state mutationsMore importantly, this state recording object replaces the multiplexing in the
internal_nodes_canvasandbuilder_multiplexerwith an object that doesn't need to broadcast the state. Instead it remembers the state itself so that if we ever switchleaf_nodes_canvasobjects (now called justcanvas) in aPlatformViewLayer, we can just update the new canvas from the internally stored state and that canvas will only see the state it needs for its current context, not all of the state from unrelated sub-trees that have already been exited. Additionally, the outgoing canvas that will no longer be the "current canvas" in the PaintContext will not see any future state changes that it has no intention of taking advantage of because it is done with its rendering.Before: all canvas objects for all embedder underly/overlay layers would see every state change call (i.e. save/Layer, etc) even if it was not going to be used in the subtree that was currently being processed in the Paint cycle.
After: each embedder underlay/overlay layer only sees the state changes that apply to the rendering it will process in its own layer
Note that this is very WIP as I have only verified host_debug_unopt compilation and haven't run any tests or apps with the changes. I am submitting this early to get the concept out for evaluation by other engineers currently working on the state changes in the layer tree. Currently it very likely destroys our opacity inheritance peephole optimizations and some way of managing those optimizations (and other saveLayer attributes) will need to be incorporated into this state object before it is ready for final reviews.