-
Notifications
You must be signed in to change notification settings - Fork 296
Description
I've been thinking about #1542 (#1447 and #1371 are also relevant to this discussion).
Right now, as we recurse through the Picture tree, we take the concatenated transform for each primitive, and calculate the screen-space rect. This is used for culling, clip-mask generation etc. This doesn't work where we have chosen to rasterize the Picture in local-space (e.g. an animated Picture, for performance reasons).
When we encounter a Picture to be drawn in local space, we need to ensure that the transforms referenced in this Picture (by clips and primitives) are relative to the root transform for this Picture, to get correct output. In this case, what we now consider the "screen-rect" would actually become a rect that is in the space of the root transform of the Picture.
If we made it so that Pictures always operate off a local collection of transforms (and we store them with the Picture), rather than referencing the global clip-scroll tree, there's a number of additional potential benefits:
- Support for Picture caching (across display lists!) becomes simpler and faster to add.
- We can compare a limited number of transforms, and they are relative to the content of the Picture, rather than the global scene.
- Many Pictures we will want to cache are effectively local-space rasterized (e.g. an image with complex filters on it), even if the calling code has requested screen-space rasterization. This will allow us to cache Pictures that are moving, but otherwise the same, for instance.
- Existing specialized support for local-space Pictures (e.g. text-shadows) is no longer needed.
- Culling can be supported in a wider range of circumstances (e.g. text-shadows).
- Some more subtle benefits - supporting proper clip masks in local-space allows us to collapse brush_line into a clip source, for example.
- This may be helpful when we start using the OS compositor interfaces.
This may sound like a lot of work, but most of it is in-place already. It's (hopefully) a medium-ish amount of refactoring, a bit of matrix math, and that's about it. Thoughts?
Activity
glennw commentedon Mar 16, 2018
I know this is quite vague, but I wanted to write something up to get some feedback. Does that make any sense? Does it sound like it would work? Are there simpler ways to achieve this that I'm missing?
cc @kvark @mrobinson
glennw commentedon Mar 16, 2018
What I mean by a local collection of transforms for each Picture:
We still keep the clip-scroll tree just as it is now, but instead of building a single flat array of concatenated transforms for the GPU, we instead query the clip-scroll tree to build one of these lists for each Picture.
glennw commentedon Mar 16, 2018
(with a shared list for Pictures that are rasterized in screen-space, which is the common case)
glennw commentedon Mar 16, 2018
For reference, we already calculate a Picture-relative transform for primitives (see
webrender/webrender/src/prim_store.rs
Line 1896 in 486ee5f
glennw commentedon Mar 16, 2018
#1502 could also potentially make use of this - adding a Picture for a text run would enable them to be transparently cached once such a system is in place.
glennw commentedon Mar 18, 2018
Having thought about this a bit over the weekend, I think it's not the right solution. I need to test a couple of things this week and then write up some more details on an alternative.
kvark commentedon Mar 19, 2018
@glennw please at least tell us what's the problem with this solution
glennw commentedon Mar 20, 2018
(This is a condensed version of what @kvark and I discussed on IRC).
Obviously some of this is subjective - feedback and disagreements welcome :)
The implementation complexity of supporting local-space rasterized Pictures is somewhat high - especially to correctly handle clip masks from other coordinate systems, if they have an existing clip-chain.
The majority of the shader types we have are no slower to draw an item in screen-space then local-space. In fact, removing some of the existing local-space support in brush shaders is an optimization.
This is compatible with property animation - there's no need to rebuild the display list if a Picture has a transform animation on it.
The one thing that is significantly slower is rasterizing glyphs in screen-space. So, instead of rasterizing an entire Picture in local-space, we can rasterize it in screen-space, using local-space rasterized glyphs when the Picture is animated. The quality of the glyphs in this case will be no worse, but the quality of everything else will be improved (for example, gradients will be rendered pixel-perfect in screen-space, even when undergoing a rotation animation). We can optionally rasterize the local space glyphs at a higher resolution to improve the glyph quality in this case (in the same way we would if drawing the Picture in local-space).
WR can implicitly detect cases where a Picture can be considered as local-space compatible, draw this once, cache it in the texture cache and blit with a hardware composite during animation. For example, a Picture that contains images, text and a CSS filter, but isn't affected by complex clip-chain ancestors can be drawn in screen-space, but considered local-space compatible. Thus, it can be cached from frame to frame.
Supporting Picture caching in this way makes it more a "general" solution, allowing it to work as a general caching scheme (e.g. for text shadows, or image blurs) for Pictures across different display lists (we can do a deep comparison on the Picture contents, rather than a shallow check of the primitive indices, for example).
So, in summary:
glennw commentedon Mar 21, 2018
Follow up discussion with @mstange.
There is at least one case where drawing in screen-space does produce a noticeably worse result - if you have two rectangles, edges adjacent to each other. In the screen-space case, you'll see an anti-aliasing edge between them as they rotate.
This is probably outweighed by the other benefits (such as better AA in almost every other case, perfect gradients etc), but it is a valid concern.
Once #2551 is resolved, that will make local-space picture rendering trivial, should we decide we do want to support that.