Skip to content

Conversation

Wumpf
Copy link
Member

@Wumpf Wumpf commented Oct 3, 2025

Allows us to remove some very dreaded unsafe code \o/

Went on a rabbit-hole trip to use the fancy new map_buffer_on_submit methods. But it's actually really hard to do so because by design, our read/write belts can use a single chunk (==buffer) on multiple command encoders. Therefore, it's really hard to know which command encoder is the one that you call map_buffer_on_submit on.
I noted down my learnings in the code comments, hopefully that prevents future generations to get confused by this.

Testing

  • Windows native
  • Linux native
  • Mac native
  • WebGL
  • WebGPU

@Wumpf Wumpf added 🔺 re_renderer rendering, graphics, GPU dependencies concerning crates, pip packages etc exclude from changelog PRs with this won't show up in CHANGELOG.md labels Oct 3, 2025
@Wumpf Wumpf requested a review from Copilot October 3, 2025 09:43
Copy link

github-actions bot commented Oct 3, 2025

Web viewer built successfully.

Result Commit Link Manifest
01fa5de https://rerun.io/viewer/pr/11419 +nightly +main

Note: This comment is updated whenever you push a commit.

@Wumpf Wumpf force-pushed the andreas/update-wgpu branch from ec2576b to 7d010aa Compare October 3, 2025 09:43
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the wgpu graphics library from version 26 to 27, enabling the removal of unsafe code related to buffer mapping. The update includes API changes for error handling, device polling, and buffer management, along with additional documentation explaining limitations with the new map_buffer_on_submit feature.

Key changes:

  • Updated wgpu dependency from 26.0.1 to 27.0.0
  • Removed unsafe lifetime transmutation in CPU write GPU read buffer implementation
  • Updated API calls to match wgpu 27's new interface patterns

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Cargo.toml Updated wgpu version and removed egui "log" feature
.cargo/config.toml Added rust-lld linker configuration for Windows
crates/viewer/re_renderer/src/device_caps.rs Simplified device descriptor creation using default values
crates/viewer/re_renderer/src/context.rs Updated error callback and polling API to new wgpu 27 format
crates/viewer/re_renderer/src/allocator/cpu_write_gpu_read_belt.rs Removed unsafe lifetime transmutation and improved documentation
crates/viewer/re_renderer/src/allocator/gpu_readback_belt.rs Added documentation about map_buffer_on_submit limitations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

let write_view = buffer_slice.get_mapped_range_mut();
self.unused_offset = end_offset;

#[allow(unsafe_code)]
Copy link
Member Author

Choose a reason for hiding this comment

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

sooo happy to have this thing gone

@Wumpf Wumpf force-pushed the andreas/update-wgpu branch from 7d010aa to 3fa7196 Compare October 3, 2025 09:44
@Wumpf Wumpf force-pushed the andreas/update-wgpu branch from 3fa7196 to 0fb42a1 Compare October 3, 2025 09:46
@Wumpf
Copy link
Member Author

Wumpf commented Oct 3, 2025

Timeout was too aggressive. Need

@Wumpf
Copy link
Member Author

Wumpf commented Oct 6, 2025

@rerun-bot full-check

Copy link

github-actions bot commented Oct 6, 2025

Started a full build: https://github.com/rerun-io/rerun/actions/runs/18274497663

/// Prepare currently mapped buffers for use in a submission.
///
/// This must be called before the command encoder(s) used in [`CpuWriteGpuReadBuffer`] copy operations are submitted.
/// All existing [`CpuWriteGpuReadBuffer`] MUST be dropped before calling this function.
Copy link
Member

Choose a reason for hiding this comment

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

[VOICE OF JULES FROM PULP FICTION]
"He should have a borrow checker for this kind o' deal."

Copy link
Member

@aedm aedm left a comment

Choose a reason for hiding this comment

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

Looks solid!

@Wumpf Wumpf merged commit b39dd3c into main Oct 6, 2025
98 of 102 checks passed
@Wumpf Wumpf deleted the andreas/update-wgpu branch October 6, 2025 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies concerning crates, pip packages etc exclude from changelog PRs with this won't show up in CHANGELOG.md 🔺 re_renderer rendering, graphics, GPU
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants