Skip to content
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

Replace the wgpu_trace feature with a field in bevy_render::settings::WgpuSettings #14842

Conversation

LikeLakers2
Copy link
Contributor

@LikeLakers2 LikeLakers2 commented Aug 21, 2024

Objective

Solution

This PR performs the above objective by removing the wgpu_trace feature from all Cargo.toml files.

However, wgpu traces are still useful for debugging - but to record them, you need to pass in a directory path to store the traces in. To avoid forcing users into manually creating the renderer, bevy_render::settings::WgpuSettings now has a trace_path field, so that all of Bevy's automatic initialization can happen while still allowing for tracing.

Testing

  • Did you test these changes? If so, how?
    • I have tested these changes, but only via running cargo run -p ci. I am hoping the Github Actions workflows will catch anything I missed.
  • Are there any parts that need more testing?
    • I do not believe so.
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    • If you want to test these changes, I have updated the debugging guide (docs/debugging.md) section on WGPU Tracing.
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    • I ran the above command on a Windows 10 64-bit (x64) machine, using the stable-x86_64-pc-windows-msvc toolchain. I do not have anything set up for other platforms or targets (though I can't imagine this needs testing on other platforms).

Migration Guide

  1. The bevy/wgpu_trace, bevy_render/wgpu_trace, and bevy_internal/wgpu_trace features no longer exist. Remove them from your Cargo.toml, CI, tooling, and what-not.
  2. Follow the instructions in the updated docs/debugging.md file in the repository, under the WGPU Tracing section.

Because of the changes made, you can now generate traces to any path, rather than the hardcoded %WorkspaceRoot%/wgpu_trace (where %WorkspaceRoot% is... the root of your crate's workspace) folder.

(If WGPU hasn't restored tracing functionality...) Do note that WGPU has not yet restored tracing functionality. However, once it does, the above should be sufficient to generate new traces.

Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@TrialDragon TrialDragon added C-Docs An addition or correction to our documentation C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Aug 21, 2024
Copy link
Member

@TrialDragon TrialDragon left a comment

Choose a reason for hiding this comment

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

Some minor grammar and wording clarification, but otherwise looks pretty good to me.

docs/debugging.md Outdated Show resolved Hide resolved
docs/debugging.md Outdated Show resolved Hide resolved
docs/debugging.md Outdated Show resolved Hide resolved
LikeLakers2 and others added 4 commits August 20, 2024 23:56
…Bevy

Co-authored-by: TrialDragon <31419708+TrialDragon@users.noreply.github.com>
Co-authored-by: TrialDragon <31419708+TrialDragon@users.noreply.github.com>
Co-authored-by: TrialDragon <31419708+TrialDragon@users.noreply.github.com>
@LikeLakers2
Copy link
Contributor Author

LikeLakers2 commented Aug 21, 2024

Although I feel confident writing documentation, I am by no means a professional documentation writer - so it'd be appreciated if anybody else could give feedback to the updated WGPU Tracing guide, found in docs/debugging.md.

P.S. @TrialDragon Thank you for the feedback!

Copy link
Member

@TrialDragon TrialDragon left a comment

Choose a reason for hiding this comment

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

Looks good to me, docs-wise.

Copy link
Contributor

@Olle-Lukowski Olle-Lukowski left a comment

Choose a reason for hiding this comment

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

Docs look great!

@Olle-Lukowski Olle-Lukowski 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 Aug 21, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Aug 25, 2024
Merged via the queue into bevyengine:main with commit 94d40d2 Aug 25, 2024
31 checks passed
@LikeLakers2 LikeLakers2 deleted the feature/bevy_render/move_wgpu_trace_to_wgpusettings branch August 25, 2024 16:50
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-Docs An addition or correction to our documentation C-Feature A new feature, making something new possible D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Consider what to do with wgpu_trace feature
5 participants