Store picture primitives in a vec, and refer by index in prims. #3213
Conversation
This is prep work for changing the structure of the prepare_prims pass. Currently, the code is (roughly): - Traverse primitive tree, recursing through pictures. - When visible, call prepare_prim to update GPU cache, clips etc. - Assign render tasks to passes. - Assign primitive lists for each picture to batches. In future, it will instead be: - Traverse picture tree, but not individual primitives. - Determine surface allocation, plane-splitting, visibility. - For each visible picture, prepare_prims and add to batches. This will simplify some existing code, as well as making possible some requirements for picture caching (e.g. determining the scale to rasterize a picture at before creating the child render tasks).
|
r? @kvark This is a start point to what we were discussing earlier. A few points:
|
There was a problem hiding this comment.
Looks good in general!
Traverse picture tree, but not individual primitives.
but we still need to figure out all the local rectangles for the items, so that we know the picture rect
Reviewed 7 of 7 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gw3583)
webrender/src/display_list_flattener.rs, line 164 at r1 (raw file):
/// The root picture index for this flattener. This is the picture /// to start the culling phase from. pub root_pic_index: PictureIndex,
yay, picture indices are back!
webrender/src/prim_store.rs, line 1602 at r1 (raw file):
prim: PicturePrimitive, ) -> PictureIndex { let index = PictureIndex(self.pictures.len());
So the new code creates pictures in prepare_prim_for_render, which is done during the frame building. And the primitive store is reset at the scene building. Does that mean we can keep adding more and more pictures to the picture vector when we rebuild the frame without rebuilding the scene? This may cause us leaking memory
webrender/src/prim_store.rs, line 3073 at r1 (raw file):
} // Reset clips from previous frames since we may clip differently each frame. self.reset_clip_task(prim_instance);
why do we no longer early return from here?
There was a problem hiding this comment.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @gw3583)
webrender/src/display_list_flattener.rs, line 164 at r1 (raw file):
Previously, kvark (Dzmitry Malyshau) wrote…
yay, picture indices are back!
Yep, hindsight :)
webrender/src/prim_store.rs, line 1602 at r1 (raw file):
Previously, kvark (Dzmitry Malyshau) wrote…
So the new code creates pictures in
prepare_prim_for_render, which is done during the frame building. And the primitive store is reset at the scene building. Does that mean we can keep adding more and more pictures to the picture vector when we rebuild the frame without rebuilding the scene? This may cause us leaking memory
It shouldn't be creating pictures during prepare_prim_for_render - they should only be created during flattening?
webrender/src/prim_store.rs, line 3073 at r1 (raw file):
Previously, kvark (Dzmitry Malyshau) wrote…
why do we no longer early return from here?
Previously, this returned a specialized bool to say that a picture which had no surface would never need a clip mask, which was an optimization to avoid redundant clip mask generation.
I realized that with a change made a few weeks ago (the code above which branches on is_passthrough) that this code path never gets hit for pictures without surfaces, so it was redundant.
|
@kvark Addressed most of those comments - I'm a little unsure about the second one though. I don't think there should be any leaks, but perhaps I'm misunderstanding the comment? |
There was a problem hiding this comment.
I think we are good, thanks for clarifications!
Reviewable status:
complete! all files reviewed, all discussions resolved
|
Confident to go without a try push? Feel free to go ahead regardless ;) |
|
Closing in favor of #3218 |
This is prep work for changing the structure of the prepare_prims
pass.
Currently, the code is (roughly):
In future, it will instead be:
This will simplify some existing code, as well as making possible
some requirements for picture caching (e.g. determining the scale
to rasterize a picture at before creating the child render tasks).
This change is