Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

flipping texture coords methods has been added to the StandardMaterial #12917

Merged
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion crates/bevy_pbr/src/pbr_material.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bevy_asset::Asset;
use bevy_color::Alpha;
use bevy_math::{Affine2, Mat3, Vec4};
use bevy_math::{Affine2, Affine3, Mat2, Mat3, Vec2, Vec3, Vec4};
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_render::{
mesh::MeshVertexBufferLayoutRef, render_asset::RenderAssets, render_resource::*,
Expand Down Expand Up @@ -487,6 +487,69 @@ pub struct StandardMaterial {
pub uv_transform: Affine2,
}

impl StandardMaterial {
/// Horizontal flipping transform
///
/// Multiplying this with another Affine2 returns transformation with horizontally flipped texture coords
pub const FLIP_HORIZONTAL: Affine2 = Affine2 {
matrix2: Mat2::from_cols(Vec2::new(-1.0, 0.0), Vec2::Y),
translation: Vec2::X,
};

/// Vertical flipping transform
///
/// Multiplying this with another Affine2 returns transformation with vertically flipped texture coords
pub const FLIP_VERTICAL: Affine2 = Affine2 {
matrix2: Mat2::from_cols(Vec2::X, Vec2::new(0.0, -1.0)),
translation: Vec2::Y,
};

/// Flipping X 3D transform
///
/// Multiplying this with another Affine3 returns transformation with flipped X coords
pub const FLIP_X: Affine3 = Affine3 {
matrix3: Mat3::from_cols(Vec3::new(-1.0, 0.0, 0.0), Vec3::Y, Vec3::Z),
translation: Vec3::X,
};

/// Flipping Y 3D transform
///
/// Multiplying this with another Affine3 returns transformation with flipped Y coords
pub const FLIP_Y: Affine3 = Affine3 {
matrix3: Mat3::from_cols(Vec3::X, Vec3::new(0.0, -1.0, 0.0), Vec3::Z),
translation: Vec3::Y,
};

/// Flipping Z 3D transform
///
/// Multiplying this with another Affine3 returns transformation with flipped Z coords
pub const FLIP_Z: Affine3 = Affine3 {
matrix3: Mat3::from_cols(Vec3::X, Vec3::Y, Vec3::new(0.0, 0.0, -1.0)),
translation: Vec3::Z,
};

/// Flip the texture coordinates of the material.
pub fn flip(&mut self, horizontal: bool, vertical: bool) {
if horizontal {
// self.uv_transform *= Self::FLIP_HORIZONTAL will not work as expected
// because it would be equivalent of
// self.uv_transform = self.uv_transform * Self::FLIP_HORIZONTAL;
// which means first flip, then original uv_transform application,
// which is reverse to expected sequence
bugsweeper marked this conversation as resolved.
Show resolved Hide resolved
self.uv_transform = Self::FLIP_HORIZONTAL * self.uv_transform;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.uv_transform = Self::FLIP_HORIZONTAL * self.uv_transform;
self.uv_transform *= Self::FLIP_HORIZONTAL;

Affine implements MulAssign, so you can do this instead.

Copy link
Contributor Author

@bugsweeper bugsweeper Apr 10, 2024

Choose a reason for hiding this comment

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

I think this replacement is not correct, because MulAssign will make equivalent of self.uv_transform = self.uv_transform * Self::FLIP_HORIZONTAL; but not self.uv_transform = Self::FLIP_HORIZONTAL * self.uv_transform;. The order is important for vector math. If somebody chain flipped() for StandardMaterial instance, he would expect that flipping transform would be done after original uv_transform, not before

Copy link
Member

Choose a reason for hiding this comment

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

I see. Could you add a comment explaining why the order is important, then? I could see this being confusing to others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
if vertical {
self.uv_transform = Self::FLIP_VERTICAL * self.uv_transform;
alice-i-cecile marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// Consumes the material and returns a material with flipped texture coordinates
pub fn flipped(mut self, horizontal: bool, vertical: bool) -> Self {
self.flip(horizontal, vertical);
self
}
}

impl Default for StandardMaterial {
fn default() -> Self {
StandardMaterial {
Expand Down