[Merged by Bors] - Updated `glam` to `0.21`. #5142

CGMossa · 2022-06-29T20:42:39Z

Removed const_vec2/const_vec3
and replaced with equivalent .from_array.

Objective

Fixes #5112

Solution

encase needs to update to glam as well. See Update glam to 0.21 teoxoy/encase#4 on progress on that.
hexasphere also needs to be updated, see Updated glam to 0.21. OptimisticPeach/hexasphere#12.

Removed `const_vec2`/`const_vec3` and replaced with equivalent `.from_array`.

CGMossa · 2022-06-29T20:47:11Z

This is a PR for doing the update in #5115.

crates/bevy_pbr/src/light.rs

james7132 · 2022-06-30T01:29:29Z

Please fix the PR title: the wrong version is in there.

benches/Cargo.toml

crates/bevy_math/Cargo.toml

crates/bevy_mikktspace/Cargo.toml

tim-blackbird

As James said above, the title of this PR has a typo.

crates/bevy_pbr/src/render/light.rs

and changed to constants..

CGMossa · 2022-06-30T19:15:20Z

Could someone review my last commit? Is this too much? Should this be reverted?

Also encase has updated their glam.. So this is soon going to be relevant.

crates/bevy_pbr/src/light.rs

CGMossa · 2022-07-01T12:33:52Z

Last thing needed is that encase publishes a new version. I'll then change the version name for it, and make this PR ready.

crates/bevy_render/src/mesh/mesh/mod.rs

james7132

Other than encase and the other comments, LGTM.

`bevy_reflect`. Ooops.

teoxoy · 2022-07-03T08:36:23Z

I just released encase v0.3 (which includes the bump to glam 0.21)

CGMossa · 2022-07-03T12:19:02Z

I'm hitting some issues:

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:84:17
   |
84 | #[derive(Clone, ShaderType)]
   |                 ^^^^^^^^^^
   |                 |
   |                 could not find `Size` in `private`
   |                 in this derive macro expansion
   |
  ::: C:\Users\angus\.cargo\registry\src\github.com-1ecc6299db9ec823\encase_derive_impl-0.2.0\src\lib.rs:18:9   
   |
18 |         pub fn derive_shader_type(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
   |         ------------------------------------------------------------------------------------ in this expansion of `#[derive(ShaderType)]`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:87:5
   |
87 |     view: Mat4,
   |     ^^^^ could not find `Size` in `private`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:88:5
   |
88 |     inverse_view: Mat4,
   |     ^^^^^^^^^^^^ could not find `Size` in `private`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:89:5
   |
89 |     projection: Mat4,
   |     ^^^^^^^^^^ could not find `Size` in `private`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:90:5
   |
90 |     world_position: Vec3,
   |     ^^^^^^^^^^^^^^ could not find `Size` in `private`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:91:5
   |
91 |     width: f32,
   |     ^^^^^ could not find `Size` in `private`

error[E0433]: failed to resolve: could not find `Size` in `private`
  --> crates\bevy_render\src\view\mod.rs:92:5
   |
92 |     height: f32,
   |     ^^^^^^ could not find `Size` in `private`

error[E0405]: cannot find trait `Size` in module `encase::private`
  --> crates\bevy_render\src\view\mod.rs:84:17
   |
84 | #[derive(Clone, ShaderType)]
   |                 ^^^^^^^^^^
   |                 |
   |                 not found in `encase::private`
   |                 in this derive macro expansion
   |
  ::: C:\Users\angus\.cargo\registry\src\github.com-1ecc6299db9ec823\encase_derive_impl-0.2.0\src\lib.rs:18:9   
   |
18 |         pub fn derive_shader_type(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
   |         ------------------------------------------------------------------------------------ in this expansion of `#[derive(ShaderType)]`

Right now, I'm trying to figure out what to do with this...

teoxoy · 2022-07-03T12:27:14Z

Bevy has it's own version of encase's derive macro which has to be updated as well.

CGMossa · 2022-07-03T14:45:14Z

I'm utterly confused as to how to rebase this PR to main. So I resolved the conflicts by hand on github. I've no idea what the result of this is. I've no clue why the rebase is incredibly annoying. I cannot do it. I've tried 4 times already.

…date_glam

CGMossa · 2022-07-03T15:13:18Z

Serialisation tests are failing.

left: `"{\"type\":\"glam::f32::vec3::Vec3\",\"struct\":{\"x\":{\"type\":\"f32\",\"value\":12.0},\"y\":{\"type\":\"f32\",\"value\":3.0},\"z\":{\"type\":\"f32\",\"value\":-6.9}}}"`,

In the test code it is written as:

            assert_eq!(
                result,
                r#"{"type":"glam::vec3::Vec3","struct":{"x":{"type":"f32","value":12.0},"y":{"type":"f32","value":3.0},"z":{"type":"f32","value":-6.9}}}"#
            );

Uhm.. what is the right decision here?

tim-blackbird · 2022-07-03T15:17:53Z

Looks like the location of the type changed.
Change that part of the test assert from glam::vec3::Vec3 to glam::f32::vec3::Vec3.

CGMossa · 2022-07-03T15:25:39Z

Next issue is rounding of the number 12.

---- tests::glam::vec3_serialization stdout ----
thread 'tests::glam::vec3_serialization' panicked at 'assertion failed: `(left == right)`
  left: `"{\"type\":\"glam::f32::vec3::Vec3\",\"struct\":{\"x\":{\"type\":\"f32\",\"value\":12},\"y\":{\"type\":\"f32\",\"value\":3},\"z\":{\"type\":\"f32\",\"value\":-6.9}}}"`,
 right: `"{\"type\":\"glam::f32::vec3::Vec3\",\"struct\":{\"x\":{\"type\":\"f32\",\"value\":12.0},\"y\":{\"type\":\"f32\",\"value\":3.0},\"z\":{\"type\":\"f32\",\"value\":-6.9}}}"`', crates\bevy_reflect\src\lib.rs:923:13note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

and also the rounding of `12.0`, and `6.0`.

CGMossa · 2022-07-03T15:50:21Z

While James did say it first, I didn't notice it until now: We need a new version of hexasphere, specifically OptimisticPeach/hexasphere#12.

CGMossa · 2022-07-03T16:04:57Z

The rounding seems to be one thing locally and another thing on CI.
No idea how to deal with this.
It uses serialisation and I don't know if I can change it..

crates/bevy_render/Cargo.toml

alice-i-cecile · 2022-07-03T19:55:17Z

bors r+

Removed `const_vec2`/`const_vec3` and replaced with equivalent `.from_array`. # Objective Fixes #5112 ## Solution - `encase` needs to update to `glam` as well. See teoxoy/encase#4 on progress on that. - `hexasphere` also needs to be updated, see OptimisticPeach/hexasphere#12.

bors · 2022-07-03T20:16:03Z

Pull request successfully merged into main.

Build succeeded:

Removed `const_vec2`/`const_vec3` and replaced with equivalent `.from_array`. # Objective Fixes bevyengine#5112 ## Solution - `encase` needs to update to `glam` as well. See teoxoy/encase#4 on progress on that. - `hexasphere` also needs to be updated, see OptimisticPeach/hexasphere#12.

Updated glam to 0.2.1.

45c4cea

Removed `const_vec2`/`const_vec3` and replaced with equivalent `.from_array`.

alice-i-cecile added C-Dependencies A change to the crates that Bevy depends on A-Math Fundamental domain-agnostic mathematical operations labels Jun 29, 2022

bitshifter reviewed Jun 30, 2022

View reviewed changes

crates/bevy_pbr/src/light.rs Outdated Show resolved Hide resolved

james7132 reviewed Jun 30, 2022

View reviewed changes

benches/Cargo.toml Outdated Show resolved Hide resolved

james7132 requested changes Jun 30, 2022

View reviewed changes

crates/bevy_math/Cargo.toml Outdated Show resolved Hide resolved

crates/bevy_mikktspace/Cargo.toml Outdated Show resolved Hide resolved

james7132 mentioned this pull request Jun 30, 2022

Update glam requirement from 0.20.0 to 0.21.2 #5112

Closed

alice-i-cecile added this to the Bevy 0.8 milestone Jun 30, 2022

Feedback

9299812

tim-blackbird suggested changes Jun 30, 2022

View reviewed changes

crates/bevy_pbr/src/render/light.rs Outdated Show resolved Hide resolved

CGMossa added 3 commits June 30, 2022 17:27

Use negative Vec2 from glam.

0d5432a

cargo fmt

ef7deb2

Replaced constants

b839fd4

CGMossa changed the title ~~Updated glam to 0.2.1.~~ Updated glam to 0.21. Jun 30, 2022

Went through more code

9949e46

and changed to constants..

tim-blackbird approved these changes Jun 30, 2022

View reviewed changes

bitshifter reviewed Jun 30, 2022

View reviewed changes

crates/bevy_pbr/src/light.rs Outdated Show resolved Hide resolved

Minor edits

c2ad574

bitshifter reviewed Jul 1, 2022

View reviewed changes

crates/bevy_render/src/mesh/mesh/mod.rs Outdated Show resolved Hide resolved

james7132 approved these changes Jul 2, 2022

View reviewed changes

CGMossa added 2 commits July 2, 2022 10:28

Use splat for constant Vec3.

2f47b61

Forgot to change to 0.21 in

ce22043

`bevy_reflect`. Ooops.

Updated encase to 0.3.

5f1332b

Merge branch 'main' into update_glam

03a52c9

CGMossa added 4 commits July 3, 2022 16:52

cargo fmt

ad16dab

Merge branch 'update_glam' of https://github.com/CGMossa/bevy into up…

7dd5b96

…date_glam

This compiles.

1cc561b

cargo fmt --all

bfbc568

Location of type have changed,

70976cb

and also the rounding of `12.0`, and `6.0`.

Maybe the rounding doesn't matter?

fa730d3

CGMossa added 2 commits July 3, 2022 18:17

This tests locally

9a5174e

Updated hexasphere

54cf10a

tim-blackbird reviewed Jul 3, 2022

View reviewed changes

crates/bevy_render/Cargo.toml Outdated Show resolved Hide resolved

CGMossa added 2 commits July 3, 2022 21:27

Updated dependencies

2d6f657

hexasphere version 7.2.

3467604

alice-i-cecile approved these changes Jul 3, 2022

View reviewed changes

alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 3, 2022

bors bot changed the title ~~Updated glam to 0.21.~~ [Merged by Bors] - Updated glam to 0.21. Jul 3, 2022

bors bot closed this Jul 3, 2022

CGMossa deleted the update_glam branch July 3, 2022 20:19

mockersf mentioned this pull request Jul 4, 2022

bevy_reflect: fix broken test #5196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Updated `glam` to `0.21`. #5142

[Merged by Bors] - Updated `glam` to `0.21`. #5142

CGMossa commented Jun 29, 2022 •

edited

Loading

CGMossa commented Jun 29, 2022

james7132 commented Jun 30, 2022

tim-blackbird left a comment

CGMossa commented Jun 30, 2022

CGMossa commented Jul 1, 2022

james7132 left a comment

teoxoy commented Jul 3, 2022

CGMossa commented Jul 3, 2022

teoxoy commented Jul 3, 2022

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

tim-blackbird commented Jul 3, 2022 •

edited

Loading

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

alice-i-cecile commented Jul 3, 2022

bors bot commented Jul 3, 2022

[Merged by Bors] - Updated glam to 0.21. #5142

[Merged by Bors] - Updated glam to 0.21. #5142

Conversation

CGMossa commented Jun 29, 2022 • edited Loading

Objective

Solution

CGMossa commented Jun 29, 2022

james7132 commented Jun 30, 2022

tim-blackbird left a comment

Choose a reason for hiding this comment

CGMossa commented Jun 30, 2022

CGMossa commented Jul 1, 2022

james7132 left a comment

Choose a reason for hiding this comment

teoxoy commented Jul 3, 2022

CGMossa commented Jul 3, 2022

teoxoy commented Jul 3, 2022

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

tim-blackbird commented Jul 3, 2022 • edited Loading

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

CGMossa commented Jul 3, 2022

alice-i-cecile commented Jul 3, 2022

bors bot commented Jul 3, 2022

[Merged by Bors] - Updated `glam` to `0.21`. #5142

[Merged by Bors] - Updated `glam` to `0.21`. #5142

CGMossa commented Jun 29, 2022 •

edited

Loading

tim-blackbird commented Jul 3, 2022 •

edited

Loading