Validate that at least one index must be specified in pass timestamp writes

CHANGELOG.md

@@ -131,10 +131,11 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
 - Lower `QUERY_SET_MAX_QUERIES` (and enforced limits) from 8192 to 4096 to match WebGPU spec. By @ErichDonGubler in [#6525](https://github.com/gfx-rs/wgpu/pull/6525).
 - Allow non-filterable float on texture bindings never used with samplers when using a derived bind group layout. By @ErichDonGubler in [#6531](https://github.com/gfx-rs/wgpu/pull/6531/).
 - Replace potentially unsound usage of `PreHashedMap` with `FastHashMap`. By @jamienicol in [#6541](https://github.com/gfx-rs/wgpu/pull/6541).
-- Add missing validation for timestamp writes in compute and render passes. By @ErichDonGubler in [#6578](https://github.com/gfx-rs/wgpu/pull/6578).
+- Add missing validation for timestamp writes in compute and render passes. By @ErichDonGubler in [#6578](https://github.com/gfx-rs/wgpu/pull/6578), [#6583](https://github.com/gfx-rs/wgpu/pull/6583).
   - Check the status of the `TIMESTAMP_QUERY` feature before other validation.
   - Check that indices are in-bounds for the query set.
   - Check that begin and end indices are not equal.
+  - Check that at least one index is specified.
 - Reject destroyed buffers in query set resolution. By @ErichDonGubler in [#6579](https://github.com/gfx-rs/wgpu/pull/6579).
 
 #### Naga

wgpu-core/src/command/compute.rs

@@ -28,9 +28,7 @@ use crate::{
 use thiserror::Error;
 use wgt::{BufferAddress, DynamicOffset};
 
-use super::{
-    bind::BinderError, memory_init::CommandBufferTextureMemoryActions, SimplifiedQueryType,
-};
+use super::{bind::BinderError, memory_init::CommandBufferTextureMemoryActions};
 use crate::ray_tracing::TlasAction;
 use std::sync::Arc;
 use std::{fmt, mem::size_of, str};
@@ -310,57 +308,15 @@ impl Global {
             Err(e) => return make_err(e, arc_desc),
         };
 
-        arc_desc.timestamp_writes = if let Some(tw) = desc.timestamp_writes {
-            let &PassTimestampWrites {
-                query_set,
-                beginning_of_pass_write_index,
-                end_of_pass_write_index,
-            } = tw;
-
-            match cmd_buf
-                .device
-                .require_features(wgt::Features::TIMESTAMP_QUERY)
-            {
-                Ok(()) => (),
-                Err(e) => return make_err(e.into(), arc_desc),
-            }
-
-            let query_set = match hub.query_sets.get(query_set).get() {
-                Ok(query_set) => query_set,
-                Err(e) => return make_err(e.into(), arc_desc),
-            };
-
-            match query_set.same_device(&cmd_buf.device) {
-                Ok(()) => (),
-                Err(e) => return make_err(e.into(), arc_desc),
-            }
-
-            for idx in [beginning_of_pass_write_index, end_of_pass_write_index]
-                .into_iter()
-                .flatten()
-            {
-                match query_set.validate_query(SimplifiedQueryType::Timestamp, idx, None) {
-                    Ok(()) => (),
-                    Err(e) => return make_err(e.into(), arc_desc),
-                }
-            }
-
-            if let Some((begin, end)) = beginning_of_pass_write_index.zip(end_of_pass_write_index) {
-                if begin == end {
-                    return make_err(
-                        CommandEncoderError::TimestampWriteIndicesEqual { idx: begin },
-                        arc_desc,
-                    );
-                }
-            }
-
-            Some(ArcPassTimestampWrites {
-                query_set,
-                beginning_of_pass_write_index,
-                end_of_pass_write_index,
+        arc_desc.timestamp_writes = match desc
+            .timestamp_writes
+            .map(|tw| {
+                Self::validate_pass_timestamp_writes(&cmd_buf.device, &hub.query_sets.read(), tw)
             })
-        } else {
-            None
+            .transpose()
+        {
+            Ok(ok) => ok,
+            Err(e) => return make_err(e, arc_desc),
         };
 
         (ComputePass::new(Some(cmd_buf), arc_desc), None)

wgpu-core/src/command/mod.rs

@@ -33,7 +33,8 @@ use crate::snatch::SnatchGuard;
 
 use crate::init_tracker::BufferInitTrackerAction;
 use crate::ray_tracing::{BlasAction, TlasAction};
-use crate::resource::{InvalidResourceError, Labeled};
+use crate::resource::{Fallible, InvalidResourceError, Labeled, ParentDevice as _, QuerySet};
+use crate::storage::Storage;
 use crate::track::{DeviceTracker, Tracker, UsageScope};
 use crate::LabelHelpers;
 use crate::{api_log, global::Global, id, resource_log, Label};
@@ -660,6 +661,8 @@ pub enum CommandEncoderError {
     TimestampWriteIndicesEqual { idx: u32 },
     #[error(transparent)]
     TimestampWritesInvalid(#[from] QueryUseError),
+    #[error("no begin or end indices were specified for pass timestamp writes, expected at least one to be set")]
+    TimestampWriteIndicesMissing,
 }
 
 impl Global {
@@ -780,6 +783,50 @@ impl Global {
         }
         Ok(())
     }
+
+    fn validate_pass_timestamp_writes(
+        device: &Device,
+        query_sets: &Storage<Fallible<QuerySet>>,
+        timestamp_writes: &PassTimestampWrites,
+    ) -> Result<ArcPassTimestampWrites, CommandEncoderError> {
+        let &PassTimestampWrites {
+            query_set,
+            beginning_of_pass_write_index,
+            end_of_pass_write_index,
+        } = timestamp_writes;
+
+        device.require_features(wgt::Features::TIMESTAMP_QUERY)?;
+
+        let query_set = query_sets.get(query_set).get()?;
+
+        query_set.same_device(device)?;
+
+        for idx in [beginning_of_pass_write_index, end_of_pass_write_index]
+            .into_iter()
+            .flatten()
+        {
+            query_set.validate_query(SimplifiedQueryType::Timestamp, idx, None)?;
+        }
+
+        if let Some((begin, end)) = beginning_of_pass_write_index.zip(end_of_pass_write_index) {
+            if begin == end {
+                return Err(CommandEncoderError::TimestampWriteIndicesEqual { idx: begin });
+            }
+        }
+
+        if beginning_of_pass_write_index
+            .or(end_of_pass_write_index)
+            .is_none()
+        {
+            return Err(CommandEncoderError::TimestampWriteIndicesMissing);
+        }
+
+        Ok(ArcPassTimestampWrites {
+            query_set,
+            beginning_of_pass_write_index,
+            end_of_pass_write_index,
+        })
+    }
 }
 
 fn push_constant_clear<PushFn>(offset: u32, size_bytes: u32, mut push_fn: PushFn)

wgpu-core/src/command/render.rs

@@ -1,7 +1,6 @@
 use crate::binding_model::BindGroup;
 use crate::command::{
     validate_and_begin_occlusion_query, validate_and_begin_pipeline_statistics_query,
-    SimplifiedQueryType,
 };
 use crate::init_tracker::BufferInitTrackerAction;
 use crate::pipeline::RenderPipeline;
@@ -1392,42 +1391,10 @@ impl Global {
                     None
                 };
 
-            arc_desc.timestamp_writes = if let Some(tw) = desc.timestamp_writes {
-                let &PassTimestampWrites {
-                    query_set,
-                    beginning_of_pass_write_index,
-                    end_of_pass_write_index,
-                } = tw;
-
-                let query_set = query_sets.get(query_set).get()?;
-
-                device.require_features(wgt::Features::TIMESTAMP_QUERY)?;
-
-                query_set.same_device(device)?;
-
-                for idx in [beginning_of_pass_write_index, end_of_pass_write_index]
-                    .into_iter()
-                    .flatten()
-                {
-                    query_set.validate_query(SimplifiedQueryType::Timestamp, idx, None)?;
-                }
-
-                if let Some((begin, end)) =
-                    beginning_of_pass_write_index.zip(end_of_pass_write_index)
-                {
-                    if begin == end {
-                        return Err(CommandEncoderError::TimestampWriteIndicesEqual { idx: begin });
-                    }
-                }
-
-                Some(ArcPassTimestampWrites {
-                    query_set,
-                    beginning_of_pass_write_index,
-                    end_of_pass_write_index,
-                })
-            } else {
-                None
-            };
+            arc_desc.timestamp_writes = desc
+                .timestamp_writes
+                .map(|tw| Global::validate_pass_timestamp_writes(device, &query_sets, tw))
+                .transpose()?;
 
             arc_desc.occlusion_query_set =
                 if let Some(occlusion_query_set) = desc.occlusion_query_set {