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

Change type of bytes_per_row and rows_per_image #3529

Merged
merged 2 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ By @teoxoy in [#3534](https://github.com/gfx-rs/wgpu/pull/3534)

- Change type of `mip_level_count` and `array_layer_count` (members of `TextureViewDescriptor` and `ImageSubresourceRange`) from `Option<NonZeroU32>` to `Option<u32>`. By @teoxoy in [#3445](https://github.com/gfx-rs/wgpu/pull/3445)
- All `fxhash` dependencies have been replaced with `rustc-hash`. By @james7132 in [#3502](https://github.com/gfx-rs/wgpu/pull/3502)
- Change type of `bytes_per_row` and `rows_per_image` (members of `ImageDataLayout`) from `Option<NonZeroU32>` to `Option<u32>`. By @teoxoy in [#3529](https://github.com/gfx-rs/wgpu/pull/3529)

### Changes

Expand Down
9 changes: 4 additions & 5 deletions deno_webgpu/src/command_encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use deno_core::ResourceId;
use serde::Deserialize;
use std::borrow::Cow;
use std::cell::RefCell;
use std::num::NonZeroU32;
use std::rc::Rc;

use super::error::WebGpuResult;
Expand Down Expand Up @@ -291,8 +290,8 @@ pub fn op_webgpu_command_encoder_copy_buffer_to_texture(
buffer: source_buffer_resource.1,
layout: wgpu_types::ImageDataLayout {
offset: source.offset,
bytes_per_row: NonZeroU32::new(source.bytes_per_row.unwrap_or(0)),
rows_per_image: NonZeroU32::new(source.rows_per_image.unwrap_or(0)),
bytes_per_row: source.bytes_per_row,
rows_per_image: source.rows_per_image,
},
};
let destination = wgpu_core::command::ImageCopyTexture {
Expand Down Expand Up @@ -339,8 +338,8 @@ pub fn op_webgpu_command_encoder_copy_texture_to_buffer(
buffer: destination_buffer_resource.1,
layout: wgpu_types::ImageDataLayout {
offset: destination.offset,
bytes_per_row: NonZeroU32::new(destination.bytes_per_row.unwrap_or(0)),
rows_per_image: NonZeroU32::new(destination.rows_per_image.unwrap_or(0)),
bytes_per_row: destination.bytes_per_row,
rows_per_image: destination.rows_per_image,
},
};
gfx_ok!(command_encoder => instance.command_encoder_copy_texture_to_buffer(
Expand Down
6 changes: 2 additions & 4 deletions deno_webgpu/src/queue.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

use std::num::NonZeroU32;

use deno_core::error::AnyError;
use deno_core::op;
use deno_core::OpState;
Expand Down Expand Up @@ -55,8 +53,8 @@ impl From<GpuImageDataLayout> for wgpu_types::ImageDataLayout {
fn from(layout: GpuImageDataLayout) -> Self {
wgpu_types::ImageDataLayout {
offset: layout.offset,
bytes_per_row: NonZeroU32::new(layout.bytes_per_row.unwrap_or(0)),
rows_per_image: NonZeroU32::new(layout.rows_per_image.unwrap_or(0)),
bytes_per_row: layout.bytes_per_row,
rows_per_image: layout.rows_per_image,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{num::NonZeroU32, ops::Range};
use std::ops::Range;

#[cfg(feature = "trace")]
use crate::device::trace::Command as TraceCommand;
Expand Down Expand Up @@ -363,7 +363,7 @@ fn clear_texture_via_buffer_copies<A: hal::Api>(
zero_buffer_copy_regions.push(hal::BufferTextureCopy {
buffer_layout: wgt::ImageDataLayout {
offset: 0,
bytes_per_row: NonZeroU32::new(bytes_per_row),
bytes_per_row: Some(bytes_per_row),
rows_per_image: None,
},
texture_base: hal::TextureCopyBase {
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/command/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub(crate) fn validate_linear_texture_data(
let bytes_in_last_row = width_in_blocks * block_size;

let bytes_per_row = if let Some(bytes_per_row) = layout.bytes_per_row {
let bytes_per_row = bytes_per_row.get() as BufferAddress;
let bytes_per_row = bytes_per_row as BufferAddress;
if bytes_per_row < bytes_in_last_row {
return Err(TransferError::InvalidBytesPerRow);
}
Expand All @@ -276,7 +276,7 @@ pub(crate) fn validate_linear_texture_data(
0
};
let block_rows_per_image = if let Some(rows_per_image) = layout.rows_per_image {
let rows_per_image = rows_per_image.get() as BufferAddress;
let rows_per_image = rows_per_image as BufferAddress;
if rows_per_image < height_in_blocks {
return Err(TransferError::InvalidRowsPerImage);
}
Expand Down
29 changes: 12 additions & 17 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
use hal::{CommandEncoder as _, Device as _, Queue as _};
use parking_lot::Mutex;
use smallvec::SmallVec;
use std::{iter, mem, num::NonZeroU32, ptr};
use std::{iter, mem, ptr};
use thiserror::Error;

/// Number of command buffers that we generate from the same pool
Expand Down Expand Up @@ -649,15 +649,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let width_blocks = size.width / block_width;
let height_blocks = size.height / block_height;

let block_rows_per_image = match data_layout.rows_per_image {
Some(rows_per_image) => rows_per_image.get(),
None => {
// doesn't really matter because we need this only if we copy
// more than one layer, and then we validate for this being not
// None
size.height
}
};
let block_rows_per_image = data_layout.rows_per_image.unwrap_or(
// doesn't really matter because we need this only if we copy
// more than one layer, and then we validate for this being not
// None
size.height,
);

let block_size = dst
.desc
Expand Down Expand Up @@ -738,11 +735,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.as_raw()
.ok_or(TransferError::InvalidTexture(destination.texture))?;

let bytes_per_row = if let Some(bytes_per_row) = data_layout.bytes_per_row {
bytes_per_row.get()
} else {
width_blocks * block_size
};
let bytes_per_row = data_layout
.bytes_per_row
.unwrap_or(width_blocks * block_size);

// Platform validation requires that the staging buffer always be
// freed, even if an error occurs. All paths from here must call
Expand Down Expand Up @@ -796,8 +791,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
offset: rel_array_layer as u64
* block_rows_per_image as u64
* stage_bytes_per_row as u64,
bytes_per_row: NonZeroU32::new(stage_bytes_per_row),
rows_per_image: NonZeroU32::new(block_rows_per_image),
bytes_per_row: Some(stage_bytes_per_row),
rows_per_image: Some(block_rows_per_image),
},
texture_base,
size: hal_copy_size,
Expand Down
6 changes: 2 additions & 4 deletions wgpu-hal/examples/halmark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use raw_window_handle::{HasRawDisplayHandle, HasRawWindowHandle};

use std::{
borrow::{Borrow, Cow},
iter, mem,
num::NonZeroU32,
ptr,
iter, mem, ptr,
time::Instant,
};

Expand Down Expand Up @@ -328,7 +326,7 @@ impl<A: hal::Api> Example<A> {
let copy = hal::BufferTextureCopy {
buffer_layout: wgt::ImageDataLayout {
offset: 0,
bytes_per_row: NonZeroU32::new(4),
bytes_per_row: Some(4),
rows_per_image: None,
},
texture_base: hal::TextureCopyBase {
Expand Down
17 changes: 7 additions & 10 deletions wgpu-hal/src/dx12/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,19 +33,16 @@ impl crate::BufferTextureCopy {
Height: self
.buffer_layout
.rows_per_image
.map_or(self.size.height, |count| count.get() * block_height),
.map_or(self.size.height, |count| count * block_height),
Depth: self.size.depth,
RowPitch: {
let actual = match self.buffer_layout.bytes_per_row {
Some(count) => count.get(),
let actual = self.buffer_layout.bytes_per_row.unwrap_or_else(|| {
// this may happen for single-line updates
None => {
let block_size = format
.block_size(Some(self.texture_base.aspect.map()))
.unwrap();
(self.size.width / block_width) * block_size
}
};
let block_size = format
.block_size(Some(self.texture_base.aspect.map()))
.unwrap();
(self.size.width / block_width) * block_size
});
crate::auxil::align_to(actual, d3d12_ty::D3D12_TEXTURE_DATA_PITCH_ALIGNMENT)
},
},
Expand Down
12 changes: 6 additions & 6 deletions wgpu-hal/src/gles/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,11 +597,11 @@ impl super::Queue {
let row_texels = copy
.buffer_layout
.bytes_per_row
.map_or(0, |bpr| block_width * bpr.get() / block_size);
.map_or(0, |bpr| block_width * bpr / block_size);
let column_texels = copy
.buffer_layout
.rows_per_image
.map_or(0, |rpi| block_height * rpi.get());
.map_or(0, |rpi| block_height * rpi);

unsafe { gl.bind_texture(dst_target, Some(dst)) };
unsafe { gl.pixel_store_i32(glow::UNPACK_ROW_LENGTH, row_texels as i32) };
Expand Down Expand Up @@ -711,13 +711,13 @@ impl super::Queue {
let bytes_per_row = copy
.buffer_layout
.bytes_per_row
.map_or(copy.size.width * block_size, |bpr| bpr.get());
.unwrap_or(copy.size.width * block_size);
let minimum_rows_per_image =
(copy.size.height + block_height - 1) / block_height;
let rows_per_image = copy
.buffer_layout
.rows_per_image
.map_or(minimum_rows_per_image, |rpi| rpi.get());
.unwrap_or(minimum_rows_per_image);

let bytes_per_image = bytes_per_row * rows_per_image;
let minimum_bytes_per_image = bytes_per_row * minimum_rows_per_image;
Expand Down Expand Up @@ -819,11 +819,11 @@ impl super::Queue {
let row_texels = copy
.buffer_layout
.bytes_per_row
.map_or(copy.size.width, |bpr| bpr.get() / block_size);
.map_or(copy.size.width, |bpr| bpr / block_size);
let column_texels = copy
.buffer_layout
.rows_per_image
.map_or(copy.size.height, |bpr| bpr.get());
.unwrap_or(copy.size.height);

unsafe { gl.bind_framebuffer(glow::READ_FRAMEBUFFER, Some(self.copy_fbo)) };

Expand Down
14 changes: 4 additions & 10 deletions wgpu-hal/src/metal/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,11 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
.texture_base
.max_copy_size(&dst.copy_size)
.min(&copy.size);
let bytes_per_row = copy
.buffer_layout
.bytes_per_row
.map_or(0, |v| v.get() as u64);
let bytes_per_row = copy.buffer_layout.bytes_per_row.unwrap_or(0) as u64;
let image_byte_stride = if extent.depth > 1 {
copy.buffer_layout
.rows_per_image
.map_or(0, |v| v.get() as u64 * bytes_per_row)
.map_or(0, |v| v as u64 * bytes_per_row)
} else {
// Don't pass a stride when updating a single layer, otherwise metal validation
// fails when updating a subset of the image due to the stride being larger than
Expand Down Expand Up @@ -277,14 +274,11 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
.texture_base
.max_copy_size(&src.copy_size)
.min(&copy.size);
let bytes_per_row = copy
.buffer_layout
.bytes_per_row
.map_or(0, |v| v.get() as u64);
let bytes_per_row = copy.buffer_layout.bytes_per_row.unwrap_or(0) as u64;
let bytes_per_image = copy
.buffer_layout
.rows_per_image
.map_or(0, |v| v.get() as u64 * bytes_per_row);
.map_or(0, |v| v as u64 * bytes_per_row);
encoder.copy_from_texture_to_buffer(
&src.raw,
copy.texture_base.array_layer as u64,
Expand Down
4 changes: 2 additions & 2 deletions wgpu-hal/src/vulkan/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ impl super::Texture {
let block_size = format
.block_size(Some(r.texture_base.aspect.map()))
.unwrap();
block_width * (bpr.get() / block_size)
block_width * (bpr / block_size)
}),
buffer_image_height: r
.buffer_layout
.rows_per_image
.map_or(0, |rpi| rpi.get() * block_height),
.map_or(0, |rpi| rpi * block_height),
image_subresource,
image_offset,
image_extent: conv::map_copy_extent(&extent),
Expand Down
4 changes: 2 additions & 2 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5222,7 +5222,7 @@ pub struct ImageDataLayout {
/// [CEcbtt]: ../wgpu/struct.CommandEncoder.html#method.copy_buffer_to_texture
/// [CEcttb]: ../wgpu/struct.CommandEncoder.html#method.copy_texture_to_buffer
/// [Qwt]: ../wgpu/struct.Queue.html#method.write_texture
pub bytes_per_row: Option<NonZeroU32>,
pub bytes_per_row: Option<u32>,
/// "Rows" that make up a single "image".
///
/// A row is one row of pixels or of compressed blocks in the x direction.
Expand All @@ -5232,7 +5232,7 @@ pub struct ImageDataLayout {
/// The amount of rows per image may be larger than the actual amount of rows of data.
///
/// Required if there are multiple images (i.e. the depth is more than one).
pub rows_per_image: Option<NonZeroU32>,
pub rows_per_image: Option<u32>,
}

/// Specific type of a buffer binding.
Expand Down
2 changes: 1 addition & 1 deletion wgpu/examples/bunnymark/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ impl framework::Example for Example {
&buf,
wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: std::num::NonZeroU32::new(info.width * 4),
bytes_per_row: Some(info.width * 4),
rows_per_image: None,
},
size,
Expand Down
5 changes: 1 addition & 4 deletions wgpu/examples/capture/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,7 @@ async fn create_red_image_with_dimensions(
buffer: &output_buffer,
layout: wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: Some(
std::num::NonZeroU32::new(buffer_dimensions.padded_bytes_per_row as u32)
.unwrap(),
),
bytes_per_row: Some(buffer_dimensions.padded_bytes_per_row as u32),
rows_per_image: None,
},
},
Expand Down
2 changes: 1 addition & 1 deletion wgpu/examples/cube/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl framework::Example for Example {
&texels,
wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: Some(std::num::NonZeroU32::new(size).unwrap()),
bytes_per_row: Some(size),
rows_per_image: None,
},
texture_extent,
Expand Down
4 changes: 2 additions & 2 deletions wgpu/examples/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ pub struct FrameworkRefTest {
#[cfg(test)]
#[allow(dead_code)]
pub fn test<E: Example>(mut params: FrameworkRefTest) {
use std::{mem, num::NonZeroU32};
use std::mem;

assert_eq!(params.width % 64, 0, "width needs to be aligned 64");

Expand Down Expand Up @@ -592,7 +592,7 @@ pub fn test<E: Example>(mut params: FrameworkRefTest) {
buffer: &dst_buffer,
layout: wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: NonZeroU32::new(params.width * 4),
bytes_per_row: Some(params.width * 4),
rows_per_image: None,
},
},
Expand Down
4 changes: 2 additions & 2 deletions wgpu/examples/mipmap/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
mod framework;

use bytemuck::{Pod, Zeroable};
use std::{borrow::Cow, f32::consts, mem, num::NonZeroU32};
use std::{borrow::Cow, f32::consts, mem};
use wgpu::util::DeviceExt;

const TEXTURE_FORMAT: wgpu::TextureFormat = wgpu::TextureFormat::Rgba8UnormSrgb;
Expand Down Expand Up @@ -247,7 +247,7 @@ impl framework::Example for Example {
buffer: &temp_buf,
layout: wgpu::ImageDataLayout {
offset: 0,
bytes_per_row: Some(NonZeroU32::new(4 * size).unwrap()),
bytes_per_row: Some(4 * size),
rows_per_image: None,
},
},
Expand Down
Loading