Add ConcreteShape type

[PoignardAzur]

Supersedes #331.
Based on @ratmice's code in that PR.

[Philipp-M]

You may want to look at https://github.com/linebender/xilem/pull/591/files#diff-d503d474ddd46b17b3dcb3674bf88c1654f893c21e81bbf2cf2098196b804e58R362

To avoid the boxed iterator.

[PoignardAzur]

Oh, right, this did seem more inefficient than I remembered.

[ratmice]

I personally have no use for this code with the type modified this way since it is now a closed enum, rather than my original patch which was an open enum due to the generic parameter. This makes the type not useful for a library, when the library is intended to be used by a crate that implements the Shape trait.

[PoignardAzur]

We'll see in practice whether people have use for it. I already know it's going to be useful in Masonry.

[ratmice]

There is nothing here trait wise which requires the implementation to be in kurbo itself. IMO without supporting downstream implementations of the Shape trait if this is useful in Masonry, it is better to just put the implementation in Masonry. Would it's usefulness there even require it being pub?

[waywardmonkeys]

I also don't really understand what problem in masonry this is going to be solving to know how to evaluate that.

Why can't some experimentation happen with a version of this within masonry for now since we would otherwise have to do a release of Kurbo and hope that the API was both right and useful?

[PoignardAzur]

Well we could always stick to the version introduced in linebender/xilem#591

[ratmice]

I definitely feel that I would prefer downstream crates adopting ad-hoc solutions over
inclusion of a solution in kurbo that only works with Kurbo included implementations of the trait.

That is to say, that this is working around an issue with the Shape trait not being object-safe,
The proposition with object safety is something like forall t: Trait, ObjectSafe(t) -> exists dyn Trait.
This patch by being limited to kurbo specific variants for the Shape impls, doesn't apply in all the cases that forall would.

Thus my feeling is that this patch doesn't fix the problem, it only fixes the common occurrences of the problem.
In doing so, it would seem to kill any motivation towards actually fixing the problem...

I would rather not see this included lightly based on it's convenience,
Fixing it in downstream libraries like masonry isn't convenient or ideal,
it in fact impedes code sharing across libraries that depend upon kurbo,
but ensures that there is motivation to fix it properly, and avoids hamstringing kurbo to something silly should we decide to re-adopt object safety for the trait or some other option...

Apologies if i'm being overly difficult in this regard...

[PoignardAzur]

Overall, there's been little enough enthusiasm for this feature that I'd rather close the PR for now.