Skip to content

Add render diagnostic functions for reading from a buffer #22326

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.

Sign up for GitHub

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

Jump to bottom
Open
JMS55 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add render diagnostic functions for reading from a buffer #22326

JMS55 wants to merge 4 commits into from

Conversation

@JMS55
Copy link
Contributor

@JMS55 JMS55 commented Dec 31, 2025

See release note file.

Tested locally on a modified copy of Solari.

@JMS55 JMS55 added A-Rendering Drawing game state to the screen A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Dec 31, 2025
@JMS55 JMS55 added this to the 0.19 milestone Dec 31, 2025
@JMS55 JMS55 requested a review from IceSentry December 31, 2025 18:08
@JMS55 JMS55 requested a review from tychedelia December 31, 2025 20:21
tychedelia
tychedelia approved these changes Dec 31, 2025
View reviewed changes
Copy link
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

not blocking but would be nice to add vec2/3/4. thanks this is great!

crates/bevy_render/src/diagnostic/internal.rs
name: Cow<'static, str>,
is_f32: bool,
) {
let dest_buffer = self.device.create_buffer(&BufferDescriptor {
Copy link
Member

@tychedelia tychedelia Dec 31, 2025

Choose a reason for hiding this comment

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

probs not a huge deal since this is debug code but we could probably re-use these allocations

Copy link
Contributor Author

@JMS55 JMS55 Jan 1, 2026

Choose a reason for hiding this comment

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

Yeah we could reuse them or suballocate from one larger buffer, but I didn't want to deal with the complexity of that... Decided to go the dumb route for now.

@alice-i-cecile alice-i-cecile added S-Needs-Review Needs reviewer attention (from anyone!) to move forward M-Release-Note Work that should be called out in the blog due to impact X-Uncontroversial This work is generally agreed upon C-Feature A new feature, making something new possible labels Dec 31, 2025
@JMS55
Copy link
Contributor Author

JMS55 commented Jan 1, 2026

not blocking but would be nice to add vec2/3/4. thanks this is great!

Sadly bevy diagnostics only supports a single f64 value :/

pcwalton
pcwalton approved these changes Jan 2, 2026
View reviewed changes
Copy link
Contributor

@pcwalton pcwalton left a comment

Choose a reason for hiding this comment

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

Yes please. Would be nice to have i32 support as well but obviously we shouldn't block the PR on that.

@JMS55
Copy link
Contributor Author

JMS55 commented Jan 2, 2026 •
edited
Loading

Oh whoops I forgot about i32 😅 . I can add that.

Actually going to punt on i32, it's rarely used in graphics contexts.

@JMS55 JMS55 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 Jan 2, 2026
@JMS55 JMS55 mentioned this pull request Jan 2, 2026
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IceSentry IceSentry Awaiting requested review from IceSentry

2 more reviewers

@pcwalton pcwalton pcwalton approved these changes

@tychedelia tychedelia tychedelia approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible M-Release-Note Work that should be called out in the blog due to impact 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

Rendering
Status: No status

Milestone

0.19

Development

Successfully merging this pull request may close these issues.

4 participants

@JMS55 @pcwalton @tychedelia @alice-i-cecile