Skip to content

Commit

Permalink
Reformat comments in wgpu-core. (#3102)
Browse files Browse the repository at this point in the history
  • Loading branch information
jimblandy authored Oct 13, 2022
1 parent fa4d840 commit 2158841
Show file tree
Hide file tree
Showing 22 changed files with 525 additions and 289 deletions.
22 changes: 15 additions & 7 deletions wgpu-core/src/binding_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,9 @@ pub struct BindGroupEntry<'a> {
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct BindGroupDescriptor<'a> {
/// Debug label of the bind group. This will show up in graphics debuggers for easy identification.
/// Debug label of the bind group.
///
/// This will show up in graphics debuggers for easy identification.
pub label: Label<'a>,
/// The [`BindGroupLayout`] that corresponds to this bind group.
pub layout: BindGroupLayoutId,
Expand All @@ -416,7 +418,9 @@ pub struct BindGroupDescriptor<'a> {
#[cfg_attr(feature = "trace", derive(serde::Serialize))]
#[cfg_attr(feature = "replay", derive(serde::Deserialize))]
pub struct BindGroupLayoutDescriptor<'a> {
/// Debug label of the bind group layout. This will show up in graphics debuggers for easy identification.
/// Debug label of the bind group layout.
///
/// This will show up in graphics debuggers for easy identification.
pub label: Label<'a>,
/// Array of entries in this BindGroupLayout
pub entries: Cow<'a, [wgt::BindGroupLayoutEntry]>,
Expand Down Expand Up @@ -537,16 +541,20 @@ pub enum PushConstantUploadError {
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct PipelineLayoutDescriptor<'a> {
/// Debug label of the pipeine layout. This will show up in graphics debuggers for easy identification.
/// Debug label of the pipeine layout.
///
/// This will show up in graphics debuggers for easy identification.
pub label: Label<'a>,
/// Bind groups that this pipeline uses. The first entry will provide all the bindings for
/// "set = 0", second entry will provide all the bindings for "set = 1" etc.
pub bind_group_layouts: Cow<'a, [BindGroupLayoutId]>,
/// Set of push constant ranges this pipeline uses. Each shader stage that uses push constants
/// must define the range in push constant memory that corresponds to its single `layout(push_constant)`
/// uniform block.
/// Set of push constant ranges this pipeline uses. Each shader stage that
/// uses push constants must define the range in push constant memory that
/// corresponds to its single `layout(push_constant)` uniform block.
///
/// If this array is non-empty, the [`Features::PUSH_CONSTANTS`](wgt::Features::PUSH_CONSTANTS) must be enabled.
/// If this array is non-empty, the
/// [`Features::PUSH_CONSTANTS`](wgt::Features::PUSH_CONSTANTS) feature must
/// be enabled.
pub push_constant_ranges: Cow<'a, [wgt::PushConstantRange]>,
}

Expand Down
5 changes: 3 additions & 2 deletions wgpu-core/src/command/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,9 @@ struct PushConstantChange {
enable: bool,
}

/// Break up possibly overlapping push constant ranges into a set of non-overlapping ranges
/// which contain all the stage flags of the original ranges. This allows us to zero out (or write any value)
/// Break up possibly overlapping push constant ranges into a set of
/// non-overlapping ranges which contain all the stage flags of the
/// original ranges. This allows us to zero out (or write any value)
/// to every possible value.
pub fn compute_nonoverlapping_ranges(
ranges: &[wgt::PushConstantRange],
Expand Down
26 changes: 18 additions & 8 deletions wgpu-core/src/command/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,28 @@ use hal::CommandEncoder as _;
#[cfg_attr(feature = "trace", derive(serde::Serialize))]
#[cfg_attr(feature = "replay", derive(serde::Deserialize))]
pub struct RenderBundleEncoderDescriptor<'a> {
/// Debug label of the render bundle encoder. This will show up in graphics debuggers for easy identification.
/// Debug label of the render bundle encoder.
///
/// This will show up in graphics debuggers for easy identification.
pub label: Label<'a>,
/// The formats of the color attachments that this render bundle is capable to rendering to. This
/// must match the formats of the color attachments in the renderpass this render bundle is executed in.
/// The formats of the color attachments that this render bundle is capable
/// to rendering to.
///
/// This must match the formats of the color attachments in the
/// renderpass this render bundle is executed in.
pub color_formats: Cow<'a, [Option<wgt::TextureFormat>]>,
/// Information about the depth attachment that this render bundle is capable to rendering to. The format
/// must match the format of the depth attachments in the renderpass this render bundle is executed in.
/// Information about the depth attachment that this render bundle is
/// capable to rendering to.
///
/// The format must match the format of the depth attachments in the
/// renderpass this render bundle is executed in.
pub depth_stencil: Option<wgt::RenderBundleDepthStencil>,
/// Sample count this render bundle is capable of rendering to. This must match the pipelines and
/// the renderpasses it is used in.
/// Sample count this render bundle is capable of rendering to.
///
/// This must match the pipelines and the renderpasses it is used in.
pub sample_count: u32,
/// If this render bundle will rendering to multiple array layers in the attachments at the same time.
/// If this render bundle will rendering to multiple array layers in the
/// attachments at the same time.
pub multiview: Option<NonZeroU32>,
}

Expand Down
32 changes: 23 additions & 9 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,19 @@ pub(crate) fn clear_texture<A: HalApi>(
layers: range.layer_range.clone(),
};

// If we're in a texture-init usecase, we know that the texture is already tracked since whatever caused the init requirement,
// will have caused the usage tracker to be aware of the texture. Meaning, that it is safe to call call change_replace_tracked if the life_guard is already gone
// (i.e. the user no longer holds on to this texture).
// On the other hand, when coming via command_encoder_clear_texture, the life_guard is still there since in order to call it a texture object is needed.
// If we're in a texture-init usecase, we know that the texture is already
// tracked since whatever caused the init requirement, will have caused the
// usage tracker to be aware of the texture. Meaning, that it is safe to
// call call change_replace_tracked if the life_guard is already gone (i.e.
// the user no longer holds on to this texture).
//
// We could in theory distinguish these two scenarios in the internal clear_texture api in order to remove this check and call the cheaper change_replace_tracked whenever possible.
// On the other hand, when coming via command_encoder_clear_texture, the
// life_guard is still there since in order to call it a texture object is
// needed.
//
// We could in theory distinguish these two scenarios in the internal
// clear_texture api in order to remove this check and call the cheaper
// change_replace_tracked whenever possible.
let dst_barrier = texture_tracker
.set_single(storage, dst_texture_id.0, selector, clear_usage)
.unwrap()
Expand Down Expand Up @@ -332,8 +339,13 @@ fn clear_texture_via_buffer_copies<A: hal::Api>(
// round down to a multiple of rows needed by the texture format
let max_rows_per_copy = max_rows_per_copy / format_desc.block_dimensions.1 as u32
* format_desc.block_dimensions.1 as u32;
assert!(max_rows_per_copy > 0, "Zero buffer size is too small to fill a single row of a texture with format {:?} and desc {:?}",
texture_desc.format, texture_desc.size);
assert!(
max_rows_per_copy > 0,
"Zero buffer size is too small to fill a single row \
of a texture with format {:?} and desc {:?}",
texture_desc.format,
texture_desc.size
);

let z_range = 0..(if texture_desc.dimension == wgt::TextureDimension::D3 {
mip_size.depth_or_array_layers
Expand All @@ -344,7 +356,8 @@ fn clear_texture_via_buffer_copies<A: hal::Api>(
for array_layer in range.layer_range.clone() {
// TODO: Only doing one layer at a time for volume textures right now.
for z in z_range.clone() {
// May need multiple copies for each subresource! However, we assume that we never need to split a row.
// May need multiple copies for each subresource! However, we
// assume that we never need to split a row.
let mut num_rows_left = mip_size.height;
while num_rows_left > 0 {
let num_rows = num_rows_left.min(max_rows_per_copy);
Expand Down Expand Up @@ -400,7 +413,8 @@ fn clear_texture_via_render_passes<A: hal::Api>(
for mip_level in range.mip_range {
let extent = extent_base.mip_level_size(mip_level, is_3d_texture);
let layer_or_depth_range = if dst_texture.desc.dimension == wgt::TextureDimension::D3 {
// TODO: We assume that we're allowed to do clear operations on volume texture slices, this is not properly specified.
// TODO: We assume that we're allowed to do clear operations on
// volume texture slices, this is not properly specified.
0..extent.depth_or_array_layers
} else {
range.layer_range.clone()
Expand Down
13 changes: 9 additions & 4 deletions wgpu-core/src/command/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ impl<A: HalApi> State<A> {
Ok(())
}

// `extra_buffer` is there to represent the indirect buffer that is also part of the usage scope.
// `extra_buffer` is there to represent the indirect buffer that is also
// part of the usage scope.
fn flush_states(
&mut self,
raw_encoder: &mut A::CommandEncoder,
Expand Down Expand Up @@ -391,7 +392,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
raw.begin_compute_pass(&hal_desc);
}

// Immediate texture inits required because of prior discards. Need to be inserted before texture reads.
// Immediate texture inits required because of prior discards. Need to
// be inserted before texture reads.
let mut pending_discard_init_fixups = SurfacesInDiscardState::new();

for command in base.commands {
Expand Down Expand Up @@ -763,8 +765,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
cmd_buf.status = CommandEncoderStatus::Recording;

// There can be entries left in pending_discard_init_fixups if a bind group was set, but not used (i.e. no Dispatch occurred)
// However, we already altered the discard/init_action state on this cmd_buf, so we need to apply the promised changes.
// There can be entries left in pending_discard_init_fixups if a bind
// group was set, but not used (i.e. no Dispatch occurred)
//
// However, we already altered the discard/init_action state on this
// cmd_buf, so we need to apply the promised changes.
fixup_discarded_surfaces(
pending_discard_init_fixups.into_iter(),
raw,
Expand Down
76 changes: 54 additions & 22 deletions wgpu-core/src/command/memory_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ pub(crate) type SurfacesInDiscardState = Vec<TextureSurfaceDiscard>;

#[derive(Default)]
pub(crate) struct CommandBufferTextureMemoryActions {
// init actions describe the tracker actions that we need to be executed before the command buffer is executed
/// The tracker actions that we need to be executed before the command
/// buffer is executed.
init_actions: Vec<TextureInitTrackerAction>,
// discards describe all the discards that haven't been followed by init again within the command buffer
// i.e. everything in this list resets the texture init state *after* the command buffer execution
/// All the discards that haven't been followed by init again within the
/// command buffer i.e. everything in this list resets the texture init
/// state *after* the command buffer execution
discards: Vec<TextureSurfaceDiscard>,
}

Expand All @@ -54,19 +56,22 @@ impl CommandBufferTextureMemoryActions {
) -> SurfacesInDiscardState {
let mut immediately_necessary_clears = SurfacesInDiscardState::new();

// Note that within a command buffer we may stack arbitrary memory init actions on the same texture
// Since we react to them in sequence, they are going to be dropped again at queue submit
// Note that within a command buffer we may stack arbitrary memory init
// actions on the same texture Since we react to them in sequence, they
// are going to be dropped again at queue submit
//
// We don't need to add MemoryInitKind::NeedsInitializedMemory to init_actions if a surface is part of the discard list.
// But that would mean splitting up the action which is more than we'd win here.
// We don't need to add MemoryInitKind::NeedsInitializedMemory to
// init_actions if a surface is part of the discard list. But that would
// mean splitting up the action which is more than we'd win here.
self.init_actions
.extend(match texture_guard.get(action.id) {
Ok(texture) => texture.initialization_status.check_action(action),
Err(_) => return immediately_necessary_clears, // texture no longer exists
});

// We expect very few discarded surfaces at any point in time which is why a simple linear search is likely best.
// (i.e. most of the time self.discards is empty!)
// We expect very few discarded surfaces at any point in time which is
// why a simple linear search is likely best. (i.e. most of the time
// self.discards is empty!)
let init_actions = &mut self.init_actions;
self.discards.retain(|discarded_surface| {
if discarded_surface.texture == action.id
Expand All @@ -79,7 +84,9 @@ impl CommandBufferTextureMemoryActions {
if let MemoryInitKind::NeedsInitializedMemory = action.kind {
immediately_necessary_clears.push(discarded_surface.clone());

// Mark surface as implicitly initialized (this is relevant because it might have been uninitialized prior to discarding
// Mark surface as implicitly initialized (this is relevant
// because it might have been uninitialized prior to
// discarding
init_actions.push(TextureInitTrackerAction {
id: discarded_surface.texture,
range: TextureInitRange {
Expand All @@ -99,7 +106,8 @@ impl CommandBufferTextureMemoryActions {
immediately_necessary_clears
}

// Shortcut for register_init_action when it is known that the action is an implicit init, not requiring any immediate resource init.
// Shortcut for register_init_action when it is known that the action is an
// implicit init, not requiring any immediate resource init.
pub(crate) fn register_implicit_init<A: hal::Api>(
&mut self,
id: id::Valid<TextureId>,
Expand All @@ -118,7 +126,9 @@ impl CommandBufferTextureMemoryActions {
}
}

// Utility function that takes discarded surfaces from (several calls to) register_init_action and initializes them on the spot.
// Utility function that takes discarded surfaces from (several calls to)
// register_init_action and initializes them on the spot.
//
// Takes care of barriers as well!
pub(crate) fn fixup_discarded_surfaces<
A: HalApi,
Expand Down Expand Up @@ -148,14 +158,16 @@ pub(crate) fn fixup_discarded_surfaces<
}

impl<A: HalApi> BakedCommands<A> {
// inserts all buffer initializations that are going to be needed for executing the commands and updates resource init states accordingly
// inserts all buffer initializations that are going to be needed for
// executing the commands and updates resource init states accordingly
pub(crate) fn initialize_buffer_memory(
&mut self,
device_tracker: &mut Tracker<A>,
buffer_guard: &mut Storage<Buffer<A>, id::BufferId>,
) -> Result<(), DestroyedBufferError> {
// Gather init ranges for each buffer so we can collapse them.
// It is not possible to do this at an earlier point since previously executed command buffer change the resource init state.
// It is not possible to do this at an earlier point since previously
// executed command buffer change the resource init state.
let mut uninitialized_ranges_per_buffer = FastHashMap::default();
for buffer_use in self.buffer_memory_init_actions.drain(..) {
let buffer = buffer_guard
Expand Down Expand Up @@ -194,15 +206,19 @@ impl<A: HalApi> BakedCommands<A> {
// Collapse touching ranges.
ranges.sort_by_key(|r| r.start);
for i in (1..ranges.len()).rev() {
assert!(ranges[i - 1].end <= ranges[i].start); // The memory init tracker made sure of this!
// The memory init tracker made sure of this!
assert!(ranges[i - 1].end <= ranges[i].start);
if ranges[i].start == ranges[i - 1].end {
ranges[i - 1].end = ranges[i].end;
ranges.swap_remove(i); // Ordering not important at this point
}
}

// Don't do use_replace since the buffer may already no longer have a ref_count.
// However, we *know* that it is currently in use, so the tracker must already know about it.
// Don't do use_replace since the buffer may already no longer have
// a ref_count.
//
// However, we *know* that it is currently in use, so the tracker
// must already know about it.
let transition = device_tracker
.buffers
.set_single(buffer_guard, buffer_id, hal::BufferUses::COPY_DST)
Expand All @@ -223,8 +239,20 @@ impl<A: HalApi> BakedCommands<A> {
}

for range in ranges.iter() {
assert!(range.start % wgt::COPY_BUFFER_ALIGNMENT == 0, "Buffer {:?} has an uninitialized range with a start not aligned to 4 (start was {})", raw_buf, range.start);
assert!(range.end % wgt::COPY_BUFFER_ALIGNMENT == 0, "Buffer {:?} has an uninitialized range with an end not aligned to 4 (end was {})", raw_buf, range.end);
assert!(
range.start % wgt::COPY_BUFFER_ALIGNMENT == 0,
"Buffer {:?} has an uninitialized range with a start \
not aligned to 4 (start was {})",
raw_buf,
range.start
);
assert!(
range.end % wgt::COPY_BUFFER_ALIGNMENT == 0,
"Buffer {:?} has an uninitialized range with an end \
not aligned to 4 (end was {})",
raw_buf,
range.end
);

unsafe {
self.encoder.clear_buffer(raw_buf, range.clone());
Expand All @@ -234,8 +262,10 @@ impl<A: HalApi> BakedCommands<A> {
Ok(())
}

// inserts all texture initializations that are going to be needed for executing the commands and updates resource init states accordingly
// any textures that are left discarded by this command buffer will be marked as uninitialized
// inserts all texture initializations that are going to be needed for
// executing the commands and updates resource init states accordingly any
// textures that are left discarded by this command buffer will be marked as
// uninitialized
pub(crate) fn initialize_texture_memory(
&mut self,
device_tracker: &mut Tracker<A>,
Expand Down Expand Up @@ -290,7 +320,9 @@ impl<A: HalApi> BakedCommands<A> {
}
}

// Now that all buffers/textures have the proper init state for before cmdbuf start, we discard init states for textures it left discarded after its execution.
// Now that all buffers/textures have the proper init state for before
// cmdbuf start, we discard init states for textures it left discarded
// after its execution.
for surface_discard in self.texture_memory_actions.discards.iter() {
let texture = texture_guard
.get_mut(surface_discard.texture)
Expand Down
Loading

0 comments on commit 2158841

Please sign in to comment.