Move the transform related boilerplate in Xilem to a single View

[DJMcNab]

Also significantly reduces the overhead from each View for supporting transforms.

Follow-up from #806

[Philipp-M]

I'm still a little bit undecided about this, I've certainly thought about something like this when implementing #753, as noted in #806, because I'm also not keen of introducing boilerplate and slower code.

My focus there was entirely on the API-side, which I think for now is more important until obvious perf issues arise (which I think currently are mostly in masonry).

I'd really like to avoid having to "transform" views into a view that disallows further use of the respective functionality of the widget.
And that something like button().transformable().transformable() is not possible, which I think is certainly something that can happen e.g. when wrapped inside a component, which I think should be supported, and I even would like to avoid having to write .transformable() at all.

But this is just my opinion, so I don't want to block on that, if others see it different.
Code seems to look good otherwise.

I think we might want to think about a more fundamental refactor of this kind of stuff (view composition) in the future though anyway, which addresses all these kinds of issues, ideally only having to "pay" for actual modified attributes of a widget.
But that might be tricky if we want to keep it simpler than in xilem_web...

[Philipp-M]

Thanks that's definitely more bearable.
The point of not being able to modify attributes of the underlying widget or being able to return the opaque type still stands, but I think that could be follow-up PRs (as previously stated) with as you mentioned in #806 bounds like WidgetView<Widget=SomeConcreteType>.

[Philipp-M]

To avoid this extra call, we could also reintroduce the extension trait again with slightly different functionality, i.e. add this Transformed view directly for each of these modifiers, which I think could now be done.

[Philipp-M]

That could of course also be directly in the WidgetView trait.

[DJMcNab]

What do you mean? The extra call is only here for demonstration purposes - it's not part of the API?

I think having it be extension trait methods would be bad, because it would be easy to get nested Transformed views, which should be avoided most of the time.

[Philipp-M]

What do you mean?

Being able to write:

sized_box(status)
    .translate(self.translation)
    .rotate(self.rotation)
    .scale(self.scale);

instead of

sized_box(status)
    .transformed()
    .translate(self.translation)
    .rotate(self.rotation)
    .scale(self.scale);

which should be avoided most of the time.

You mean that it's less performant or that it "hides" that this is a separate type, and that the widgets properties can't be modified afterwards anymore?

Out of a technical/performance oriented view, it's probably less optimal, than just using one Transformed and modifying it in place, but I don't think this is really relevant in practice (we need sophisticated bench/profiling to prove this though).

Though I think it would definitely look cleaner, and in practice I don't think that a lot of nesting will occur anyway (this example is probably already in the upper range).

[DJMcNab]

You mean that it's less performant

Yes, it would be a performance footgun which isn't necessary.

The fewer extension traits/methods there are, the better, because it (e.g.) complicates autocomplete.

[Philipp-M]

Yes, it would be a performance footgun which isn't necessary.

Is it really? It's just copying a little bit of data on the stack mostly isn't it?

because it (e.g.) complicates autocomplete.

Hmm, actually I think it does the opposite (probably a matter of taste?).
Taking xilem_web as example, I kind of like, that rust-analyzer suggests e.g. for an input element (input(()).|) something like:

checked(...) (as HtmlInputElement)
required(...) (as HtmlInputElement)
attr(...) (as Element)
class(...) (as Element)

So that it's directly obvious what is possible to configure with the widget ("flattened", without indirection, like transformed()), and showing the "context" (trait, in the example the relevant DOM interface). Kind of self-documenting, just by scrolling through the completion menu.

[DJMcNab]

Is it really? It's just copying a little bit of data on the stack mostly isn't it?

Alongside doing a matrix multiplication on widget creation, and doing a comparison between two affine transformations on every rebuild, doing a matrix multiplication whenever it changes, and doing an additional matrix multiplication when the child changes, storing extra data in the state tree (and as you say, copying a little bit of data on the stack). In the grand scheme of things it's not that much, but it's so unnecessary to do it this way only to make things slightly cleaner when using a very niche API.

If every element has five properties intermingled in the autocomplete list which almost no-one will use, it seems like that would hide the actually interesting properties e.g. (checked, required) in that example case.

The clear compromise here would be to expose only .transform, which doesn't have "indirection", whilst not splatter-gunning items throughout the autocomplete list. If people want the builder-style API, we can point them to the main transformed.

[Philipp-M]

The clear compromise here would be to expose only .transform

Yeah might be a good compromise, also in the regard, that it "opens" a new transform "context" within the parentheses (so rustfmt might produce better (i.e. indented) code? And maybe the optimizer can optimize the given Affine parameter even better?).

In the grand scheme of things it's not that much, but it's so unnecessary to do it this way only to make things slightly cleaner when using a very niche API.

Well, I guess it depends on what the focus is (and how much performance impact it really has in practice). My goal/concern would mostly be, in case we're moving towards a full-composition API, it would be streamlined then (similar as in xilem_web), using the same pattern throughout the API.

Just to be clear, The main concern (panic, when composing) is solved, so IMO I think it's good.

[DJMcNab]

Technically this could fail if the child transform happened to exactly add our old value, and we don't change.

I'm not sure how to handle that - maybe we can piggyback off the transform_changed flag from Masonry?

[Philipp-M]

maybe we can piggyback off the transform_changed flag from Masonry?

Sounds like an idea, would probably slightly simplify View::rebuild as well.

[Philipp-M]

Just a formal approval, thanks for bearing with me and being patient through these lengthy discussions...