Skip to content

Packed Vals#24800

Open
ickshonpe wants to merge 7 commits into
bevyengine:mainfrom
ickshonpe:pack-val
Open

Packed Vals#24800
ickshonpe wants to merge 7 commits into
bevyengine:mainfrom
ickshonpe:pack-val

Conversation

@ickshonpe

@ickshonpe ickshonpe commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Objective

A Val is 8 bytes, which can't realistically be reduced without using f16s which aren't accurate enough.

However with Val2, UiRect and BorderRadius you can combine the Val enum discriminators into one value. The reduces
Val2 from 16 bytes down to 12, and UiRect/BorderRadius goes from 32 bytes down to 20 bytes. If BorderRadius is extended to support elliptical radii (#24779) it would need 64 bytes which could be packed down to 36 bytes.

Solution

  • pack and unpack functions on Val, map it to and from a (u8, f32) pair.
  • Changed Val2 to use packed Vals.

  • The discriminator might be better as a #[repr(u8)] enum with explicit values, though then you'd have to do a match or transmute to unpack.

  • It might not a be a very popular change, I'm not quite sure how well it works API wise. Maybe something could be done with bsn! though.

let d = direction * 50.0 * time.delta_secs();
let (Val::Px(x), Val::Px(y)) = (transform.translation.x, transform.translation.y)
let (Val::Px(x), Val::Px(y)) =
(transform.translation.x(), transform.translation.y())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that Val2 is user-facing, making the fields not directly set-able seems like an undesirable UX tradeoff to make in the interest of satisfying what appears to be renderer size requirements?

I think I'd prefer a conversion at the point where that size requirement manifests rather than eating the UX cost everywhere

@ickshonpe ickshonpe Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about data to the renderer. Responsive coords are completely resolved before we send anything to the renderer, it's just sent physical render coordinates.

What's bothering me, and maybe I'm fussing unnecessarly, is the footprint per UI node entity. Users will want to create complex UIs that feature things like game maps, graphs, inventory screens, etc which can easily have tens of thousands of nodes. So with a huge Node struct we have poorer performance updating and traversing the tree due to poor cache locality. I haven't done much research or benchmarking though, and our layout performance is fine most of time.

With Val2 in particular, I don't think the UX is that much worse (For UiRect it would be much worse, which is why I started with Val2). For construction, types like Val2 using struct literal syntax is quite rare compared to using the builder functions. And for updating values, responsive coords already makes things clumsy, it's not like with Vec2 where you can do my_vec2.x += 10. as the units might not be compatible. So you almost always want to use a helper function or deconstruct for update operations.

My naive idea was to followup this up by deriving FromTemplate for Node and add Val2Template, UiRectTemplate and BorderRadiusTemplate types, like:

pub struct Val2Template {
    pub x: Val,
    pub y: Val,
}

impl Default for Val2Template {
    fn default() -> Self {
        Self::from(Val2::default())
    }
}

impl From<Val2> for Val2Template {
    fn from(value: Val2) -> Self {
        Self {
            x: value.x(),
            y: value.y(),
        }
    }
}

impl Template for Val2Template {
    type Output = Val2;

    fn build_template(&self, _context: &mut TemplateContext) -> bevy_ecs::error::Result<Val2> {
        Ok(Val2::new(self.x, self.y))
    }

    fn clone_template(&self) -> Self {
        Self {
            x: self.x,
            y: self.y,
        }
    }
}

Then we can still use the name field syntax and decrease the size. My intuition wrt what's possible with bsn! and what the limitations are isn't that great yet though, maybe it's a bad idea or there are other options.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd want very compelling benchmark results before embracing something like this in the interest of performance. Its not just a UX issue: this introduces more code + complexity to maintain and understand.

@ickshonpe ickshonpe added the A-UI Graphical user interfaces, styles, layouts, and widgets label Jun 29, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in UI Jun 29, 2026
@ickshonpe ickshonpe added C-Performance A change motivated by improving speed, memory usage or compile times X-Contentious There are nontrivial implications that should be thought through D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jun 29, 2026
@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 29, 2026
@IceSentry

Copy link
Copy Markdown
Contributor

Do we have any kind of benchmarks that shows this kind of packing is needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Performance A change motivated by improving speed, memory usage or compile times D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help X-Contentious There are nontrivial implications that should be thought through

Projects

Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

4 participants