bevyengine / bevyengine/bevy

Bug in the `Mul` impl for `Transform` and `GlobalTransform`

Open
#4,910 13 comments 0 reactions 0 assignees View on GitHub
A-Transform C-Bug
Dominant language
Rust
Stars
48.2k
Forks
4.8k
Avg merge
3d 22h
Merged PRs (30d)
161

Description

## Bevy version

I have tested both the current master as of this writing (9976ecb) and v0.7.0.

## What you did

I think there is a bug in the `Mul` implementation of `Transform` and `GlobalTransform`. I would expect these two expressions to be equivalent (up to rounding errors):

```rust
let t1 = Transform::from_scale(Vec3::new(2.0, 1.0, 1.0));
let quat = Quat::from_rotation_z(TAU / 4.0);
let t2 = Transform::from_rotation(quat)
* Transform::from_scale(Vec3::new(1.0, 2.0, 1.0))
* Transform::from_rotation(quat.inverse());
// t1 and t2 are not the same
```

I would expect this, because computing the same transform using matrix multiplications yields the expected result:

```rust
let t3 = Transform::from_matrix(
Transform::from_rotation(quat).compute_matrix()
* Transform::from_scale(Vec3::new(1.0, 2.0, 1.0)).compute_matrix()
* Transform::from_rotation(quat.inverse()).compute_matrix(),
);
// t1 and t3 are the same up to rounding errors
```

## What went wrong

I was expecting the two transforms to be equivalent up to rounding point errors. However `t2` ends up scaling the `y`-axis instead of the `x`-axis.

## Additional information

I believe the bug is in `mul_transform` where the scales from the two transforms are multiplied together without taking the rotation into account.

I think there is a similar bug in `rotate` and `rotate_around` as well.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.