Skip to content

Add inverse model matrix to Mesh uniform buffer#6608

Closed
pbaja wants to merge 2 commits intobevyengine:mainfrom
pbaja:add-inverse-model-matrix
Closed

Add inverse model matrix to Mesh uniform buffer#6608
pbaja wants to merge 2 commits intobevyengine:mainfrom
pbaja:add-inverse-model-matrix

Conversation

@pbaja
Copy link

@pbaja pbaja commented Nov 14, 2022

Objective

  • Enable users to convert positions from world to object local space in shaders. This is often useful when f.eg. ray marching.
  • Match unity_WorldToObject functionality from Unity

Solution

  • Add model_inverse field to Mesh and Mesh2d structs
  • Compute and assign the variable to MeshUniform and Mesh2dUniform. This is free because we are already computing the inverse model matrix for the inverse_transpose_model field.
  • Additional change was to rename the field from transform to model in the MeshUniform and Mesh2dUniform structs, to match structs in shaders and reduce confusion. I can revert it if this is unwanted.

@james7132 james7132 added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen labels Nov 14, 2022
@james7132 james7132 requested a review from superdump November 14, 2022 12:10
@james7132
Copy link
Member

james7132 commented Nov 14, 2022

I'd like to see some performance numbers here. MeshUniform's size in memory is notorious for impacting end-to-end rendering performance. Particularly since IIRC, matrix transposes are basically free on modern GPU hardware.

@james7132 james7132 added the S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help label Nov 14, 2022
@pbaja
Copy link
Author

pbaja commented Nov 14, 2022

I have run many_cubes test with cargo run --example many_cubes --release sphere.

AdapterInfo { name: "NVIDIA GeForce RTX 3060 Laptop GPU", vendor: 4318, device: 9504, device_type: DiscreteGpu, driver: "NVIDIA", driver_info: "517.48", backend: Vulkan }
Diagnostic Before change After Change
Frametime 23 - 26 ms 24 - 27 ms
Framerate 38 - 42 FPS 34 - 40 FPS

Not great.

In this case, I guess to get a model inverse matrix, one should do let inverse_model = transpose(mesh.inverse_transpose_model); which should be pretty cheap.

I guess this PR can be closed.

@james7132
Copy link
Member

james7132 commented Nov 14, 2022

It may have worth to move the transpose explicitly to the GPU. If it's "free" on the GPU and has a cost on the CPU, makes more sense to just provide the inverse and transpose it in the shader. It's also a simpler mental model for developers.

@james7132
Copy link
Member

@pbaja do you mind trying the alternative I proposed above? If it shows that it's as performant, we can probably merge the change.

@atlv24
Copy link
Contributor

atlv24 commented May 4, 2024

Now that #12773 is merged, this can be closed. The model inverse transpose is computed on the GPU anyways due to bandwidth savings. Getting inverse model is just a different unpacking of it and should be pretty cheap.

@superdump
Copy link
Contributor

I wouldn't say that it can be closed only because of #12773 because that is only addresses platforms with compute. That said, the inverse transpose is available and can be transposed back and reconstructed into the full inverse model matrix on all platforms, and various performance optimisations have and continue to be done to make performance as good as it reasonably can be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants