Layers that modify transforms

[timtom-dev]

Currently, push_layer() doesn't modify the transforms of drawing operations in that layer, but there are a couple places in my project where that would be useful. I could append() the fragment that I'd like to transform, but that involves a copy and it may be very large, so I'd rather avoid that if possible.

I think there's two obvious options to change the API to allow this. Either, add an Option<Transform> parameter to push_layer() that gets applied to everything within that layer, or add a separate function called something like push_transform_layer() that only effects transforms. In the second case it might make sense to rename push_layer() to push_clip_layer(), but then you'd have two different types of layers to pop, which could be confusing and error prone.

If this is something the project is willing to include, I'm happy to implement whatever API gets settled on! The scene encoding doesn't seem too hard to work with... (knock on wood)

[DJMcNab]

The main difficulty here is that currently transforms are just inserted as-is, and this would require a second book-keeping layer of transform stacks, which would only be used for this API. Would you consider creating your own version of this for external use. e.g.

pub struct TransformedScene {
    scene: &mut Scene,
    transform: Affine
}
impl TransformedScene {
    pub fn fill(...){self.scene.fill(..., self.transform.then(transform))}
}

[timtom-dev]

I suppose that would work, but I'm exposing the scene to client code in a GUI framework, so it would be much cleaner to have it built-in rather than creating a wrapper.

The two situations where it would be useful for me are 1) Scaling an entire scene to transparently account for HiDPI, and 2) Transforming the contents of UI nodes to the origin, so client code doesn't have to account for their final laid-out position while painting.

I imagine these two use cases are pretty useful for other GUI frameworks as well, so I would argue that it's worth an extra stack of transforms.

Again, I'm happy to implement it!

[DJMcNab]

At least in the second case, wouldn't you want the ability for scenes to be retained for a widget if it didn't need to be repainted?
That is the reason that masonry uses append for this case, which already handles this properly.
And then you can also apply the HiDPI support at the same time (although note that you probably want some of the code to be aware of the scale factor anyway, to e.g. avoid bugs like in this article).

But if you think it being handled in your paint equivalent is needed, the way to do that would is to have it handled externally. I don't see how it's any less clean to use a wrapper struct - the external API you expose would stay the same.
The implementation work isn't the problem, I just don't see that it fits with how Vello currently works.

Maybe I'm misunderstanding though. Do you have parts of our extant example code which could be written more clearly using this capability?

[timtom-dev]

If a widget wants to cache its drawing commands, there's no reason it can't store a Scene internally and append it to the main scene when it's time to draw. But for widgets that are highly dynamic, writing commands to an intermediate scene and then immediately appending to the main scene is wasteful when they could just write those commands to the main scene directly. So, in my framework I've chosen to leave that choice up to individual widgets.

The majority of my frame time is spent building the scene, which is why this added bit of flexibility is so important to me.

I would much prefer to re-export the actual Scene type from Vello, rather than a thin wrapper with an almost identical API. It would be an extra thing to keep in sync with Vello, it could be confusing to users, but also: a wrapper type wouldn't have any compatibility with the larger ecosystem of crates built on top of Vello. So that's what I mean when I say it would be less clean.

I really don't think this would be a significant departure from the API as it stands. Right now, the docs for push_layer() even emphasize that "transforms are not saved or modified by the layer stack" which I think is evidence that some users may already expect it to work this way - I know I did at first. All I'm asking for is the option.

[DJMcNab]

From my perspective this is much less clean, as it is adding extra-bookkeeping for every operation on a Scene, to slightly improve the ergonomics of this niche case.

I'm not likely to approve your proposed design, but I'm not going to block it either.
Feel free to make a PR implementing this to continue the discussion, and a different maintainer can approve if they think this adds value.

Edit: Edited to reduce snarkiness

[dfrg]

+1 to Daniel’s comments.

In addition to the extra bookkeeping, this request makes the assumption that a layer and transform group are the same which is not universally true. The color font spec, for example, does not merge these concepts and implementing that with this proposed change would be exceedingly painful.

For this reason and others, I’m strongly opposed to making this change. However, we’ve had several people ask for some form of state tracking API and it would be reasonable to add such a thing to vello. This could mimic the original piet RenderContext or the web Canvas. The semantics should be well defined and considerations should be made for conversion to/from the current scene type.

If you’re interested in working on this, I’d recommend opening a new issue or starting a zulip thread for design discussion before submitting a PR.

[timtom-dev]

Ok, so you're saying that one way forward would be to create an alternate API for constructing an Encoding? I imagine it would be built on top of Scene to reduce code duplication... Are there other discussions about that you could point me to?

How would either of you feel about exposing a &mut Encoding from Scene? I know that's probably a separate discussion, but that would also give me the flexibility to change the scale of a scene without copying the whole thing.

Sorry if at any point I came across as snarky, that wasn't my intention! I really appreciate the work you're doing. :)