bevy icon indicating copy to clipboard operation
bevy copied to clipboard

bevy_render: Centralize shader View uniform type

Open superdump opened this issue 1 year ago • 1 comments

Objective

  • Keep the shader-side definition of the ViewUniform struct in one place, in bevy_render
  • The long-term fix from #5512 and #5531

Solution

  • Move shader View struct to a shader import at bevy_render::view_types
  • Make shader view bindings for binding the view uniform to group 0 binding 0 as this is very common, at bevy_render::view_bindings - note that this itself imports bevy_render::view_types for convenience as we have done elsewhere
  • Remove the shader View struct from mesh_view_types.wgsl in bevy_pbr, mesh2d_view_types.wgsl and sprite.wgsl in bevy_sprite and ui.wgsl in bevy_ui and instead make use of the above imports.

Changelog

  • Removed: bevy_sprite::mesh2d_view_types and bevy_sprite::mesh2d_view_bindings
  • Added: bevy_render::view_types and bevy_render::view_bindings
  • Changed: All of bevy_pbr, bevy_sprite, and bevy_ui now use bevy_render::view_types and bevy_render::view_bindings to keep the shader View struct in-sync

Migration Guide

If you were using the bevy_sprite::mesh2d_view_types and/or bevy_sprite::mesh2d_view_bindings shader imports, now use bevy_render::view_types and/or bevy_render::view_bindings instead.

superdump avatar Aug 01 '22 17:08 superdump

I tested lighting, mesh2d, many_sprites, and ui and all look ok to me.

superdump avatar Aug 01 '22 17:08 superdump

Is this still relevant? Kind of a nice small change

nicopap avatar Jul 31 '23 16:07 nicopap

I forgot that this was never merged. Definitely something we want to do in my opinion because of how ViewUniform is reused.

superdump avatar Jul 31 '23 17:07 superdump

We have centralized the View type under bevy_render::view. With the new import system, it looks like #import bevy_render::view View. However binding imports are still separated inside bevy_pbr::mesh_view_bindings and bevy_sprite::mesh2d_view_bindings.

I like the idea of standardizing the view binding position and consolidating the import under bevy_render. We might also want to standardize the globals binding position to group(0) binding(1) and also place it in bevy_render?

cart avatar Aug 01 '23 19:08 cart