Details
- Reviewers
nical kvark - Commits
- rMOZILLACENTRAL784fdbec47d2: Bug 1606251 - Fix invalidation for elements with inflation factors. r=kvark
- Bugzilla Bug ID
- 1606251
Diff Detail
- Repository
- rMOZILLACENTRAL mozilla-central
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Patch looks reasonable, just one question about the approach
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2344–2374 | I find it unexpected that we'd need to iterate all the surfaces in the stack here. If a nested surface is inflated, wouldn't it be taken into account by the relevant parent surface, and therefore the surface corresponding to the tile cache space (which is the only one surface I'd expect this code to check). | |
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2344–2374 | Although the surface is inflated, it's not detected as dirty because the parameters of the filter op in the picture haven't changed, only the content in the child primitives. If we were to detect / propagate the dirtiness of a child primitive up to consider the filter picture itself dirty, then we'd be dirtying a potentially much larger region than we need to (the entire picture, rather than just the inflated portion of the child surface that was actually invalidated). When I implement child picture caching, I think handling this will probably be quite a bit simpler, since we'll be storing dependencies on each child picture, rather than at the tile cache, and it should be simpler to propagate up dirty rects. | |
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2344–2374 | I think I get it now. We are not talking about propagation of surface bounds - this is already in place and just fine. We are talking about propagation of the primitive rectangle specifically through the surfaces here. Wouldn't this be a concern for performance? The dependency updates are now O(surface_tree_depth * num_primitives), which could be heavy. | |
| gfx/wr/webrender/src/picture.rs | ||
|---|---|---|
| 2344–2374 | It's potentially a concern for performance - but this branch only runs when the primitive exists on a different surface, which is fairly rare (and the depth of the surface stack is typically quite small). Hopefully it's ok! | |