Fix resolve_queries calls per frame causing invalid buffer copy ope…

…rations (#83)

* add regression test for #82

* Fix multiple resolves per frame causing invalid buffer copy operations

src/profiler.rs

@@ -353,7 +353,8 @@ impl GpuProfiler {
                 continue;
             }
 
-            assert!(num_resolved_queries < num_used_queries);
+            debug_assert!(query_pool.capacity >= num_used_queries);
+            debug_assert!(num_resolved_queries < num_used_queries);
 
             // Resolve into offset 0 of the resolve buffer - this way we don't have to worry about
             // the offset restrictions on resolve buffers (`wgpu::QUERY_RESOLVE_BUFFER_ALIGNMENT`)
@@ -364,14 +365,16 @@ impl GpuProfiler {
                 &query_pool.resolve_buffer,
                 0,
             );
-            // Copy the resolved queries into the read buffer, making sure
+            // Copy the newly resolved queries into the read buffer, making sure
             // that we don't override any of the results that are already there.
+            let destination_offset = (num_resolved_queries * wgpu::QUERY_SIZE) as u64;
+            let copy_size = ((num_used_queries - num_resolved_queries) * wgpu::QUERY_SIZE) as u64;
             encoder.copy_buffer_to_buffer(
                 &query_pool.resolve_buffer,
                 0,
                 &query_pool.read_buffer,
-                (num_resolved_queries * wgpu::QUERY_SIZE) as u64,
-                (num_used_queries * wgpu::QUERY_SIZE) as u64,
+                destination_offset,
+                copy_size,
             );
 
             query_pool

tests/src/multiple_resolves_per_frame.rs

@@ -1,6 +1,10 @@
-// Regression test for bug described in https://github.com/Wumpf/wgpu-profiler/issues/79
+// Regression test for bug described in
+// * https://github.com/Wumpf/wgpu-profiler/issues/79
+// * https://github.com/Wumpf/wgpu-profiler/issues/82
 #[test]
 fn multiple_resolves_per_frame() {
+    const NUM_SCOPES: usize = 1000;
+
     let (_, device, queue) = super::create_device(
         wgpu::Features::TIMESTAMP_QUERY.union(wgpu::Features::TIMESTAMP_QUERY_INSIDE_ENCODERS),
     )
@@ -13,14 +17,14 @@ fn multiple_resolves_per_frame() {
         let mut encoder = device.create_command_encoder(&wgpu::CommandEncoderDescriptor::default());
 
         // Resolve call per scope.
-        {
-            let _ = profiler.scope("testscope0", &mut encoder, &device);
-        }
-        profiler.resolve_queries(&mut encoder);
-        {
-            let _ = profiler.scope("testscope1", &mut encoder, &device);
+        // Do this many times to check for potential buffer overflows as found in
+        // https://github.com/Wumpf/wgpu-profiler/issues/82
+        for i in 0..NUM_SCOPES {
+            {
+                let _ = profiler.scope(format!("{i}"), &mut encoder, &device);
+            }
+            profiler.resolve_queries(&mut encoder);
         }
-        profiler.resolve_queries(&mut encoder);
 
         // And an extra resolve for good measure (this should be a no-op).
         profiler.resolve_queries(&mut encoder);
@@ -31,8 +35,12 @@ fn multiple_resolves_per_frame() {
     // Poll to explicitly trigger mapping callbacks.
     device.poll(wgpu::Maintain::Wait);
 
-    // Frame should now be available.
-    assert!(profiler
+    // Frame should now be available and contain all the scopes.
+    let scopes = profiler
         .process_finished_frame(queue.get_timestamp_period())
-        .is_some());
+        .unwrap();
+    assert_eq!(scopes.len(), NUM_SCOPES);
+    for (i, scope) in scopes.iter().enumerate() {
+        assert_eq!(scope.label, format!("{i}"));
+    }
 }