Add AtomKind::Layout, for nesting AtomLayout#8219
Conversation
|
Preview available at https://egui-pr-preview.github.io/pr/8219-lucasatom-layout-measure-paint-split View snapshot changes at kitdiff |
| /// # }); | ||
| #[derive(Clone, Debug, Default)] | ||
| pub struct Atoms<'a>(SmallVec<[Atom<'a>; ATOMS_SMALL_VEC_SIZE]>); | ||
| pub struct Atoms<'a>(Vec<Atom<'a>>); |
There was a problem hiding this comment.
Unfortunately the nesting was impossible with smallvec due to it being invariant (whatever that means). I've run into this before.
So I switched to Vec. We could also use tinyvec or the smallvec 2.0 beta, which fixes this problem.
There was a problem hiding this comment.
This means an additional allocation for each widget 😬 Something I really would like to avoid.
Have you benchmarked the impact of this?
| pub struct AtomLayout<'a> { | ||
| id: Option<Id>, | ||
| pub atoms: Atoms<'a>, | ||
| gap: Option<f32>, | ||
| pub(crate) frame: Frame, | ||
| pub(crate) sense: Sense, | ||
| fallback_text_color: Option<Color32>, | ||
| fallback_font: Option<FontSelection>, | ||
| min_size: Vec2, | ||
| max_size: Vec2, | ||
| wrap_mode: Option<TextWrapMode>, | ||
| align2: Option<Align2>, | ||
| } | ||
|
|
There was a problem hiding this comment.
I wonder if we should also split AtomLayout into a Widget (WidgetAtom?) and a Layout (ContainerAtom?) part. Because now it kinda does both, represent how a group of Atoms should be laid out / rendered (what Margin / Frame / etc), and how it should behave as a widget (what Id / Sense should it have).
We'd then still want both a AtomKind::Container and a AtomKind::Widget, since it allocating a Response is what makes the future AtomTrait work via reading back the Response.
emilk
left a comment
There was a problem hiding this comment.
I would really like to avoid that extra allocation 😬
This allows you to put an
AtomLayoutinside anotherAtomLayout. Right now this has limited use, but once we add wrapping, verticalAtomLayoutandAtomUi, this will be a really powerful new layouting primitive for egui.Added a test for this in #8221