View Transformations

[DGriffin91]

Objective

• Add functions for common view transformations.

[alice-i-cecile]

Same comment as on #9718 (which should be coordinate with this): is there a reason why these need to be tied to PBR?

[IceSentry]

I'll answer here because I think this PR supersedes the other PR.

why these need to be tied to PBR?

Right now, bevy_pbr == bevy_3d. There's a future where we split them and pbr is strictly limited to pbr stuff but for now you can't do 3d without bevy_pbr.

[DGriffin91]

@alice-i-cecile I would love for this to live outside bevy_pbr but unfortunately with the limitations of our binding system and view_transformations's dependence on bevy_pbr::mesh_view_bindings that isn't currently possible without significantly hurting the ergonomics.

[IceSentry]

Yeah, a part of me would like to have those functions that don't assume the view bindings are present but it would indeed make using them way more annoying

[JMS55]

Please make View an argument you can pass (view: ptr<function, View>). A lot of the time you want to use these functions, you won't be using the pbr mesh view bindings.

Similarly, I don't see why we couldn't move this to bevy_render.

Please also update gtao.wgsl to use these functions and remove the old code there.

[IceSentry]

I guess one potential solution would be to have the bindings agnostic functions defined in bevy_render and the more ergonomic version defined in bevy_pbr

[DGriffin91]

@JMS55 Unfortunately you can't pass uniforms as ptr. Here are various possible solutions:

// 1. Current PR
let ws_pos = position_ndc_to_world(vec3(uv_to_ndc(uv), depth));

// 2. Pass in view ptr
var view_temp = view_bindings::view;
let view = &view_temp;
let ws_pos = position_ndc_to_world(view, vec3(uv_to_ndc(uv), depth));
    
// 3. Pass in mat
// Nothing here will keep users from entering the incorrect view field.
let ws_pos = position_ndc_to_world(view.inverse_view_proj, vec3(uv_to_ndc(uv), depth));

// 4. Compute directly but with updated view mat names
let temp = view_bindings::view.ndc_to_world * vec4(uv_to_ndc(uv), depth, 1.0);
let ws_pos = temp.xyz / temp.w;

Personally I think 1 and 4 are ok.

[JMS55]

I personally like 1. or 3. The problem is 1. doesn't work for things like SSAO. Imo, with doc comments, 3. is fine.

[DGriffin91]

3 to me is somewhat nonsensical. The contents of every position_to_* function would be exactly the same except for ones involving clip. Same for all the direction_to_* functions.

If users are going to have to read docs to use these then they can use 4.

[JMS55]

In that case I'm in favor of just 1., and at least we have this documented so I can just copy paste the code when I need it elsewhere :)

[superdump]

Please do not merge this until I have reviewed it.

[superdump]

Adding back to 0.12 milestone as a dependency of #9258 .

[superdump]

I think there is one perspective divide that is out of place.

[superdump]

This isn't a complete review pass. I'm highlighting a sticking point that we need to straighten out and align on.

[IceSentry]

Alright, I've been using the code from this PR in a couple of side projects and it worked really well.

LGTM assuming conflicts are fixed of course.