SpriteComponents with texture doesn't respect size

[naithar]

Platform:
macOS 10.15.4 / iOS 13.6

Description:
Using SpriteComponents with texture and specific size value doesn't change the way sprite is rendered.
Using only color for ColorMaterial renders sprite correctly.
Using scale works for both texture and color, but color requires size being specified.

Example:

Texture:

let texture_handle = asset_server.load("icon.png").unwrap();

commands
.spawn(Camera2dComponents::default())
.spawn(SpriteComponents {
  material: materials.add(ColorMaterial::modulated_texture(texture_handle.into(), Color::rgb(0.2, 0.2, 0.8))),
  sprite: Sprite { size: Vec2::new(50.0, 50.0) },
  ..Default::default()
});

Color:

commands
.spawn(Camera2dComponents::default())
.spawn(SpriteComponents {
  material: materials.add(Color::rgb(0.2, 0.2, 0.8).into()),
  sprite: Sprite { size: Vec2::new(50.0, 50.0) },
  ..Default::default()
});

[TheNeikos]

This is because of this system:

bevy/crates/bevy_sprite/src/sprite.rs

Lines 21 to 34 in 57177c9

	pub fn sprite_system(
	materials: Res<Assets<ColorMaterial>>,
	textures: Res<Assets<Texture>>,
	mut query: Query<(&mut Sprite, &Handle<ColorMaterial>)>,
	) {
	for (mut sprite, handle) in &mut query.iter() {
	let material = materials.get(&handle).unwrap();
	if let Some(texture_handle) = material.texture {
	if let Some(texture) = textures.get(&texture_handle) {
	sprite.size = texture.size;
	}
	}
	}
	}

[naithar]

@TheNeikos well, then I guess the question is - is this an expected behavior.
I would expect it to fire only if sprite size is not set or zero (but this also might be the value developer want for some reason), but I might be wrong :)

[TheNeikos]

I would even go as far as that only the sprite size is important and that the dev should always fill it.

[naithar]

This is because of this system:

That's the result when I remove this system, so I guess something should be done with the way this system is handled:

[naithar]

Maybe there is a way to save current behavior as default one, but also fixing it when developer specifies an actual size? Like using some trait like SpriteSize?

[TheNeikos]

I think the size could be made into an enum:

enum Size {
  Manual(f32, f32),
  Computed(f32, f32),
  Undefined,
}

This way the system seen here (#411 (comment)) will only assign a new size when it is Computed or Undefined making the intent clear. It would also default to Size::Undefined in case one doesn't want to manually size it.

An alternative would also be to mark the Sprite as being manually sized for example:

enum ResizeBehavior {
   Fixed,
   Automatic,
}

which could then be checked, and if its automatic then the size is overwritten if the Handle changes.

[naithar]

I think the size could be made into an enum:

enum Size {
  Manual(f32, f32),
  Computed(f32, f32),
  Undefined,
}

This way the system seen here (#411 (comment)) will only assign a new size when it is Computed or Undefined making the intent clear. It would also default to Size::Undefined in case one doesn't want to manually size it.

This doesn't seem to work well with shader bindings system. Just like with the traits :)

enum ResizeBehavior {
   Fixed,
   Automatic,
}

This worked pretty good.

[cart]

Closed by #430