Skip to content

Commit 633f14b

Browse files
authored
Remove offset argument from TrackedRenderPass::set_index_buffer (#20468)
# Objective - `offset` argument is misleading as the argument is not actually passed to wgpu (used only for memoization and logging). `BufferSlice` already contains an offset. - wgpu's `set_index_buffer` [sets the offset according to BufferSlice::offset](https://github.com/gfx-rs/wgpu/blob/e990388af98e4b4dff9f7fcc09a4eb5d2f71d227/wgpu/src/api/render_pass.rs#L98-L105) - `TrackedRenderPass::set_vertex_buffer` was made aware of slice size (#14916) but missed `set_index_buffer` counterpart ## Solution - Removed `offset` argument from `TrackedRenderPass::set_index_buffer` - Apply fix from #14916 to `TrackedRenderPass::is_index_buffer_set` - ~~Cleanup code by using the newly added `BufferSlice` getters~~ split out to #21289 ## Testing - Ran a few examples
1 parent 9ec9e0c commit 633f14b

File tree

11 files changed

+40
-44
lines changed

11 files changed

+40
-44
lines changed

crates/bevy_pbr/src/render/mesh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3154,7 +3154,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawMesh {
31543154
return RenderCommandResult::Skip;
31553155
};
31563156

3157-
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), 0, *index_format);
3157+
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), *index_format);
31583158

31593159
match item.extra_index() {
31603160
PhaseItemExtraIndex::None | PhaseItemExtraIndex::DynamicOffset(_) => {

crates/bevy_render/src/render_phase/draw_state.rs

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ use wgpu::{IndexFormat, QuerySet, RenderPass};
1515
#[cfg(feature = "detailed_trace")]
1616
use tracing::trace;
1717

18+
type BufferSliceKey = (BufferId, wgpu::BufferAddress, wgpu::BufferSize);
19+
1820
/// Tracks the state of a [`TrackedRenderPass`].
1921
///
2022
/// This is used to skip redundant operations on the [`TrackedRenderPass`] (e.g. setting an already
@@ -25,8 +27,8 @@ struct DrawState {
2527
pipeline: Option<RenderPipelineId>,
2628
bind_groups: Vec<(Option<BindGroupId>, Vec<u32>)>,
2729
/// List of vertex buffers by [`BufferId`], offset, and size. See [`DrawState::buffer_slice_key`]
28-
vertex_buffers: Vec<Option<(BufferId, wgpu::BufferAddress, wgpu::BufferSize)>>,
29-
index_buffer: Option<(BufferId, u64, IndexFormat)>,
30+
vertex_buffers: Vec<Option<BufferSliceKey>>,
31+
index_buffer: Option<(BufferSliceKey, IndexFormat)>,
3032

3133
/// Stores whether this state is populated or empty for quick state invalidation
3234
stores_state: bool,
@@ -87,10 +89,7 @@ impl DrawState {
8789
}
8890

8991
/// Returns the value used for checking whether `BufferSlice`s are equivalent.
90-
fn buffer_slice_key(
91-
&self,
92-
buffer_slice: &BufferSlice,
93-
) -> (BufferId, wgpu::BufferAddress, wgpu::BufferSize) {
92+
fn buffer_slice_key(&self, buffer_slice: &BufferSlice) -> BufferSliceKey {
9493
(
9594
buffer_slice.id(),
9695
buffer_slice.offset(),
@@ -99,19 +98,14 @@ impl DrawState {
9998
}
10099

101100
/// Marks the index `buffer` as bound.
102-
fn set_index_buffer(&mut self, buffer: BufferId, offset: u64, index_format: IndexFormat) {
103-
self.index_buffer = Some((buffer, offset, index_format));
101+
fn set_index_buffer(&mut self, buffer_slice: &BufferSlice, index_format: IndexFormat) {
102+
self.index_buffer = Some((self.buffer_slice_key(buffer_slice), index_format));
104103
self.stores_state = true;
105104
}
106105

107106
/// Checks, whether the index `buffer` is already bound.
108-
fn is_index_buffer_set(
109-
&self,
110-
buffer: BufferId,
111-
offset: u64,
112-
index_format: IndexFormat,
113-
) -> bool {
114-
self.index_buffer == Some((buffer, offset, index_format))
107+
fn is_index_buffer_set(&self, buffer: &BufferSlice, index_format: IndexFormat) -> bool {
108+
self.index_buffer == Some((self.buffer_slice_key(buffer), index_format))
115109
}
116110

117111
/// Resets tracking state
@@ -259,29 +253,21 @@ impl<'a> TrackedRenderPass<'a> {
259253
///
260254
/// Subsequent calls to [`TrackedRenderPass::draw_indexed`] will use the buffer referenced by
261255
/// `buffer_slice` as the source index buffer.
262-
pub fn set_index_buffer(
263-
&mut self,
264-
buffer_slice: BufferSlice<'a>,
265-
offset: u64,
266-
index_format: IndexFormat,
267-
) {
268-
if self
269-
.state
270-
.is_index_buffer_set(buffer_slice.id(), offset, index_format)
271-
{
272-
#[cfg(feature = "detailed_trace")]
273-
trace!(
274-
"set index buffer (already set): {:?} ({})",
275-
buffer_slice.id(),
276-
offset
277-
);
256+
pub fn set_index_buffer(&mut self, buffer_slice: BufferSlice<'a>, index_format: IndexFormat) {
257+
let already_set = self.state.is_index_buffer_set(&buffer_slice, index_format);
258+
#[cfg(feature = "detailed_trace")]
259+
trace!(
260+
"set index buffer{}: {:?} (offset = {}, size = {})",
261+
if already_set { " (already set)" } else { "" },
262+
buffer_slice.id(),
263+
buffer_slice.offset(),
264+
buffer_slice.size(),
265+
);
266+
if already_set {
278267
return;
279268
}
280-
#[cfg(feature = "detailed_trace")]
281-
trace!("set index buffer: {:?} ({})", buffer_slice.id(), offset);
282269
self.pass.set_index_buffer(*buffer_slice, index_format);
283-
self.state
284-
.set_index_buffer(buffer_slice.id(), offset, index_format);
270+
self.state.set_index_buffer(&buffer_slice, index_format);
285271
}
286272

287273
/// Draws primitives from the active vertex buffer(s).

crates/bevy_sprite_render/src/mesh2d/mesh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawMesh2d {
862862
return RenderCommandResult::Skip;
863863
};
864864

865-
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), 0, *index_format);
865+
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), *index_format);
866866

867867
pass.draw_indexed(
868868
index_buffer_slice.range.start..(index_buffer_slice.range.start + count),

crates/bevy_sprite_render/src/render/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,6 @@ impl<P: PhaseItem> RenderCommand<P> for DrawSpriteBatch {
947947

948948
pass.set_index_buffer(
949949
sprite_meta.sprite_index_buffer.buffer().unwrap().slice(..),
950-
0,
951950
IndexFormat::Uint32,
952951
);
953952
pass.set_vertex_buffer(

crates/bevy_ui_render/src/box_shadow.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawBoxShadow {
559559
// Store the vertices
560560
pass.set_vertex_buffer(0, vertices.slice(..));
561561
// Define how to "connect" the vertices
562-
pass.set_index_buffer(indices.slice(..), 0, IndexFormat::Uint32);
562+
pass.set_index_buffer(indices.slice(..), IndexFormat::Uint32);
563563
// Draw the vertices
564564
pass.draw_indexed(batch.range.clone(), 0, 0..1);
565565
RenderCommandResult::Success

crates/bevy_ui_render/src/gradient.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,7 +966,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawGradient {
966966
// Store the vertices
967967
pass.set_vertex_buffer(0, vertices.slice(..));
968968
// Define how to "connect" the vertices
969-
pass.set_index_buffer(indices.slice(..), 0, IndexFormat::Uint32);
969+
pass.set_index_buffer(indices.slice(..), IndexFormat::Uint32);
970970
// Draw the vertices
971971
pass.draw_indexed(batch.range.clone(), 0, 0..1);
972972
RenderCommandResult::Success

crates/bevy_ui_render/src/render_pass.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ impl<P: PhaseItem> RenderCommand<P> for DrawUiNode {
267267
// Define how to "connect" the vertices
268268
pass.set_index_buffer(
269269
indices.slice(..),
270-
0,
271270
bevy_render::render_resource::IndexFormat::Uint32,
272271
);
273272
// Draw the vertices

crates/bevy_ui_render/src/ui_texture_slice_pipeline.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawSlicer {
706706
// Store the vertices
707707
pass.set_vertex_buffer(0, vertices.slice(..));
708708
// Define how to "connect" the vertices
709-
pass.set_index_buffer(indices.slice(..), 0, IndexFormat::Uint32);
709+
pass.set_index_buffer(indices.slice(..), IndexFormat::Uint32);
710710
// Draw the vertices
711711
pass.draw_indexed(batch.range.clone(), 0, 0..1);
712712
RenderCommandResult::Success

examples/shader_advanced/custom_phase_item.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ where
9393
.buffer()
9494
.unwrap()
9595
.slice(..),
96-
0,
9796
IndexFormat::Uint32,
9897
);
9998

examples/shader_advanced/custom_shader_instancing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ impl<P: PhaseItem> RenderCommand<P> for DrawMeshInstanced {
303303
return RenderCommandResult::Skip;
304304
};
305305

306-
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), 0, *index_format);
306+
pass.set_index_buffer(index_buffer_slice.buffer.slice(..), *index_format);
307307
pass.draw_indexed(
308308
index_buffer_slice.range.start..(index_buffer_slice.range.start + count),
309309
vertex_buffer_slice.range.start as i32,

0 commit comments

Comments
 (0)