[Impeller] add basic culling checks during text frame dispatcher. #55168
Conversation
| const ContentContext& renderer_; | ||
| Matrix matrix_; | ||
| std::vector<Matrix> stack_; | ||
| std::vector<Rect> cull_rect_state_; |
There was a problem hiding this comment.
I don't think you need a stack. At least not yet. SaveLayer bounds culling already happened so using a stack so that you can incorporate that into the culling of DL dispatching isn't likely to benefit much.
DL already culled - by failing to record - anything that was distinctly outside of a clip set in the DL stream, including the bounds of SaveLayers. So trying to intersect the cull_rect as you read through the current one isn't likely to improve the culling much compared to just culling against the (inverse transformed) frame bounds. If you remove the code in SaveLayer does it change the results much for your test case? If not then you don't really need a stack here until we do a full clip-based culling and even that would be icing on the cake rather than a critical step in the process.
There was a problem hiding this comment.
We do still need to account for image filters, which aren't captured in the transform state. The locally transformed device bounds may be too small in some cases.
There was a problem hiding this comment.
Are you sure? I don't think regular display list culling takes those into account. It just forwards the inverse-mapped clip bounds to the new Dispatch call.
Keep in mind that the bounds recorded in the RTree include any expansion due the filters applied to them, even including every filter on every saveLayer that they are inside of. That kind of inverts the responsibility there, but I'd have to look at it more to be sure this is right. If it is wrong then maybe we're doing the rendering side of culling wrong as well...
There was a problem hiding this comment.
I'm sure the culling takes that into account, but this is more of a problem with the cull rect computation right? (assuming no other canvas ops) Suppose I have a NxM device rect and a saveLayer with a 2x scaling image filter on it. If I need to dispatch to a sub display list inside of that savelayer, It wouldn't be correct to use the NxM cull rect, I'd need a 2Nx2M cull rect.
For other transforms I can invert the transform matrix but we don't track the image filter transforms easily yet.
There was a problem hiding this comment.
Within a display list that has already been taken care of by the way that the RTree rects are accumulated, but I think you are correct that it is as if there is an additional "transform" that goes on when you hit a saveLayer that has a filter. So, just as you have to take the original cull rect and un-transform it by the CTM, you also have to un-transform it by the actions of the SaveLayers for similar reasons.
So, yes, it is still an issue that this code punts on the filter un-transform of the savelayers as you mentioned in the code.
| context->GetContentContext(), // | ||
| impeller::Matrix(), // | ||
| impeller::Rect::MakeSize(render_target_size) // | ||
| ); |
There was a problem hiding this comment.
This pattern of "first dispatch to a TFD and then to a DlDispatcher" happens in a number of places. It feels like a convenience method might be nice.
There was a problem hiding this comment.
In #55019 I refactored this dispatching into a reusable static method.
Fixes flutter/flutter#155133
Dl dispatching still relies on cull rects computed during that dispatch process. Make sure that the text frame dispatcher doesn't populate text frames that are way offscreen.
This culling is more conservative than the rendering dispatcher. We'd need to do some refactoring so the logic isn't repeated multiple times.