Skip to content

fix(core): validate sample_count is 1 in Queue::write_texture #7984

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

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

ErichDonGubler
Copy link
Member

This is a missing piece of the "validating texture buffer copy" routine, which is currently not outlined in the same way as the WebGPU spec.

It's almost certainly better to refactor writeTexture and other copy commands so that we can capture the routine and properly share it between different standard operations. This commit leaves this as future work, only fixing the missing validation in writeTexture.

Connections

Squash or Rebase?

Squash.

Checklist

  • If this contains user-facing changes, add a CHANGELOG.md entry.

@ErichDonGubler ErichDonGubler requested review from crowlKats and a team as code owners July 21, 2025 20:59
This is a missing piece of the ["validating texture buffer copy"
routine](https://www.w3.org/TR/webgpu/#abstract-opdef-validating-texture-buffer-copy),
which is currently not outlined in the same way as the WebGPU spec.

It's almost certainly better to refactor `writeTexture` and other copy
commands so that we can capture the routine and properly share it
between different standard operations. This commit leaves this as future
work, only fixing the missing validation in `writeTexture.`
@ErichDonGubler ErichDonGubler force-pushed the write_texture-sample-count-1 branch from d19f441 to d893b71 Compare July 21, 2025 21:00
@ErichDonGubler
Copy link
Member Author

A Rust MRE, if anyone is interested:

//! A Rust-y version of
//! [`webgpu:api,validation,queue,writeTexture:sample_count:*`](https://github.com/gpuweb/cts/blob/26530c7da1c48ea5d0bf4c5da83b15b996eb522f/src/webgpu/api/validation/queue/writeTexture.spec.ts#L60-L85).

use pollster::FutureExt as _;

fn main() {
    env_logger::init();

    let instance = wgpu::Instance::new(&wgpu::InstanceDescriptor::from_env_or_default());
    let adapter = instance
        .request_adapter(&Default::default())
        .block_on()
        .unwrap();
    let (device, queue) = adapter
        .request_device(&Default::default())
        .block_on()
        .unwrap();

    for sample_count in [1, 4] {
        let texture = device.create_texture(&wgpu::TextureDescriptor {
            label: None,
            size: wgpu::Extent3d {
                width: 16,
                height: 16,
                depth_or_array_layers: 1,
            },
            mip_level_count: 1,
            sample_count,
            dimension: wgpu::TextureDimension::D2,
            format: wgpu::TextureFormat::Bgra8Unorm,
            usage: wgpu::TextureUsages::COPY_DST | wgpu::TextureUsages::RENDER_ATTACHMENT,
            view_formats: &[],
        });

        let data = [0u8; 16];
        let size = wgpu::Extent3d {
            width: 1,
            height: 1,
            depth_or_array_layers: 1,
        };

        let is_valid = sample_count == 1;

        device.push_error_scope(wgpu::ErrorFilter::Validation);

        queue.write_texture(
            wgpu::TexelCopyTextureInfoBase {
                texture: &texture,
                mip_level: 0,
                origin: Default::default(),
                aspect: Default::default(),
            },
            &data,
            Default::default(),
            size,
        );

        match (is_valid, device.pop_error_scope().block_on()) {
            (true, None) | (false, Some(..)) => (),
            (expected_valid, err_opt) => panic!(
                concat!(
                    "expected valid = {} for `sample_count` = {}, ",
                    "but error scope yielded {:?}"
                ),
                expected_valid, sample_count, err_opt
            ),
        }
    }
}

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Jul 21, 2025

Couldyoumaybeaddthatasavalidationtestmaybe?

@andyleiserson
Copy link
Contributor

This fix is also included in #7948.

@ErichDonGubler
Copy link
Member Author

ErichDonGubler commented Jul 22, 2025

@cwfitzgerald:

Couldyoumaybeaddthatasavalidationtestmaybe?

Does…the relevant CTS test being added to CI not count? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants