Skip to content

refactor(render): move WgpuWrapper into bevy_utils #19303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2025

Conversation

atlv24
Copy link
Contributor

@atlv24 atlv24 commented May 20, 2025

Objective

  • A step towards splitting out bevy_camera from bevy_render

Solution

  • Move a shim type into bevy_utils to avoid a dependency cycle
  • Manually expand Deref/DerefMut to avoid having a bevy_derive dependency so early in the dep tree

Testing

  • It compiles

Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor changes for no_std compatibility (in case someone activates the atomics feature on wasm32v1-none for example). Otherwise, I think this is a good idea.

@bushrat011899
Copy link
Contributor

bushrat011899 commented May 20, 2025

Just need to update line 24 of bevy_utils/Cargo.toml:

- std = ["alloc", "bevy_platform/std", "dep:thread_local"]
+ std = ["alloc", "bevy_platform/std", "dep:thread_local", "dep:send_wrapper"]

@bushrat011899 bushrat011899 added C-Code-Quality A section of code that is hard to understand or change M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide A-Utils Utility functions and types X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 20, 2025
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Please review the instructions for writing migration guides, then expand or revise the content in the migration guides directory to reflect your changes.

@atlv24
Copy link
Contributor Author

atlv24 commented May 20, 2025

I don't believe this needs a migration guide, WgpuWrapper wasn't exposed before.

@bushrat011899 bushrat011899 removed the M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label May 20, 2025
@bushrat011899
Copy link
Contributor

My mistake! I saw that it was pub and assumed so.

@atlv24 atlv24 requested a review from bushrat011899 May 20, 2025 15:25
Copy link
Contributor

@bushrat011899 bushrat011899 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect!

@atlv24 atlv24 added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 20, 2025
@alice-i-cecile
Copy link
Member

@atlv24 merge conflicts, sorry!

@alice-i-cecile alice-i-cecile added this pull request to the merge queue May 27, 2025
Merged via the queue into bevyengine:main with commit 1732c22 May 27, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Utils Utility functions and types C-Code-Quality A section of code that is hard to understand or change D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants