Something wrong with custom property values in expressions used in modifiers (and maybe issue with graph::Value size too?)

[liam-b]

Crate versions
bevy version: 0.13.1
bevy_hanabi version: 0.10.0

Describe the bug
The value I'm setting for a custom property in an effect asset doesn't seem to be working properly. In the snippet below, I'm setting up a SetAttributeModifier that just writes the result of a mul involving a property to the AXIS_X attribute but something goes wrong and nothing is being displayed on the screen.

let mut module = Module::default();

// Just spawn everything in one spot
let init_pos = SetPositionCircleModifier {
    center: module.lit(Vec3::ZERO),
    axis: module.lit(Vec3::Z),
    radius: module.lit(0.0),
    dimension: ShapeDimension::Surface,
};

let prop = module.prop("my_prop");
let x_vec = module.lit(Vec3::X);
// let identity_mat = module.lit(MatrixValue::from(Mat3::IDENTITY));

// Transform the x axis with the my_prop matrix
let init_x_axis = SetAttributeModifier {
    attribute: Attribute::AXIS_X,
    value: module.mul(prop, x_vec),
    // value: module.mul(identity_mat, x_vec), // This works as you would expect
};

let effect = EffectAsset::new(32768, Spawner::rate(10.0.into()), module)
    .with_property("my_prop", Value::Matrix(Mat3::IDENTITY.into())) // Should just be identity matrix
    .init(init_pos)
    .init(init_x_axis);

commands.spawn(ParticleEffectBundle {
    effect: ParticleEffect::new(effects.add(effect)),
    ..default()
});

If you comment the value: module.mul(prop, x_vec) line out and use the one below instead, everything works fine and you can see the particles.

After poking around a bit I ran into this which makes me think it could be a layout issue? Not totally sure.

let value = Value::Matrix(Mat3::IDENTITY.into());
assert_eq!(value.as_bytes().len(), value.value_type().size());

assertion `left == right` failed
  left: 36
 right: 48

Expected behavior
To be able to use a custom property in a modifier expression for an effect asset.

To Reproduce
I made a reproduction repo over here. If you clone it and run cargo run and cargo test you should be able to see what I'm talking about.

I would dig a little deeper if I had more time right now and it's possible I'm just misunderstanding how this stuff is meant to work but hopefully someone can help me out. Thanks!

[djeedai]

Thanks @liam-b for the report and repro. I think there's two things here:

The property bug is I think a regression I saw recently. Try adding an EffectProperties component to the same entity as your ParticleEffect. I expect that might solve it. I think the code which auto-spawns it is broken.

The value size thing, I think it works as expected. When you query as_bytes() you're looking at the Rust data type on CPU (and even I think the internal representation which is private). But value_type().size() is the byte size on GPU according to WGSL layout rules, which are different from Rust. WGSL has stronger padding rules leaving more gaps for the benefit of more aligned access.

[djeedai]

Oh wait no as_bytes() is also wrong for Mat3, should be 48 bytes indeed: 3 rows of 16-byte aligned vec3.

[djeedai]

By the way to spawn everything in one place just use SetAttributeModifier(ATTRIBUTE::Position, ...) instead of a degenerate SetPositionCircleModifier. This will save a bunch of calculations which might introduce errors and NaNs due to the zero radius.

[djeedai]

Reopening to wait for confirmation about EffectProperties, but I'd like to log a separate bug for it.

[liam-b]

Yep, you seem to be right about EffectProperties not being auto spawned. But, when I add it like so, I get this error:

commands.spawn((
    ParticleEffectBundle {
        effect: ParticleEffect::new(effects.add(effect)),
        ..default()
    },
    EffectProperties::default(),
));

2024-04-03T00:37:20.082830Z  WARN bevy_hanabi: Asset  specifies motion integration but is missing Attribute::VELOCITY.
thread 'Compute Task Pool (2)' panicked at /Users/liam/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_hanabi-0.10.0/src/properties.rs:397:17:
assertion `left == right` failed
  left: 36
 right: 48
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `bevy_hanabi::render::extract_effects`!

Which looks like it comes back to the padding issue! (which is still in 0.10.0, of course)

If I change bevy_hanabi to latest from git in my dependencies it now works! Although, the issue with needing to add EffectProperties still seems to be there. Anyway, I can do what I wanted now. Thanks for your help!

[djeedai]

Yes the padding issue was probably there from the start.

I'll have a look at the EffectProperties spawning, it's probably a silly bug.

[djeedai]

Fixed by #312