gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
12.12k stars 880 forks source link

Bunnymark example does not recompute its projection matrix and other parameters when the canvas size changes #5890

Open mwyrzykowski opened 2 months ago

mwyrzykowski commented 2 months ago

Description examples/src/bunnymark/mod.rs computes an Orthographic RH matrix on page load, but if the canvas is resized, the matrix is not recomputed.

See all uses of config.width and config.height inside examples/src/bunnymark/mod.rs

Repro steps (1) Open https://wgpu.rs/examples/?backend=webgpu&example=bunnymark in Chrome 126.0.6478.127 (2) Resize the window

Expected vs observed behavior Observed: Bunnies bounce outside of the canvas or don't bounce to the top

Expected: bunnies bounce within the resized canvas

Extra materials

https://github.com/gfx-rs/wgpu/assets/105083895/a41bfd50-c2df-4200-937a-0ee0bcf3bc5a

Platform macOS using Chrome Version 128.0.6562.0 (Official Build) canary (arm64)

dv29 commented 1 month ago

Can i take this up? has there been any discovery done on this?

Wumpf commented 1 month ago

@dv29 doesn't look like, go ahead!

dv29 commented 1 month ago

I cannot replicate this issue on my machine with the following, it seems to be working as expected

OS:
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.4 LTS
Release:    22.04
Codename:   jammy

Google Chrome: Version 126.0.6478.182 (Official Build) (64-bit)
gabrieleforner commented 1 month ago

Me too, I tried even by debugging it and it works nice. Try checking your GPU manufacturer drivers.

OS Name: Apple macOS 14.5 Real. Number: 14.5 Codename: Sonoma

mwyrzykowski commented 1 month ago

@dv29 @techforni I am using Chrome Canary version 128.0.6610.0 (Official Build) canary (arm64) from macOS 15 beta and I think the issue or a similar one persists, specifically the implementation of resize is currently empty:

https://github.com/gfx-rs/wgpu/blob/101c996703ba5701cfe38efc677e76b9083592de/examples/src/bunnymark/mod.rs#L373

I would propose something along the lines of the following to examples/src/bunnymark/mod.rs, pardon my rust syntax as I have never written a line of rust before:

diff --git a/examples/src/bunnymark/mod.rs b/examples/src/bunnymark/mod.rs
index b5b33b54d..3823695d7 100644
--- a/examples/src/bunnymark/mod.rs
+++ b/examples/src/bunnymark/mod.rs
@@ -51,6 +51,7 @@ struct Example {
     local_group: wgpu::BindGroup,
     pipeline: wgpu::RenderPipeline,
     bunnies: Vec<Bunny>,
+    global_buffer: wgpu::Buffer,
     local_buffer: wgpu::Buffer,
     extent: [u32; 2],
     rng: WyRand,
@@ -75,6 +76,20 @@ impl Example {
             });
         }
     }
+    fn update_bunnies(
+        &mut self,
+        mut prior_vertical_extent: f32) {
+        if prior_vertical_extent < 1.0 {
+            prior_vertical_extent = 1.0;
+        }
+        let current_count = self.bunnies.len();
+        let new_vertical_extent = self.extent[1] as f32;
+        for i in 0..current_count {
+            let old_y = self.bunnies[i].position[1] as f32;
+            let vertical_proportion = old_y / prior_vertical_extent;
+            self.bunnies[i].position[1] = vertical_proportion * new_vertical_extent;
+        }
+    }

     fn render_inner(
         &mut self,
@@ -339,6 +354,7 @@ impl crate::framework::Example for Example {
             global_group,
             local_group,
             bunnies: Vec::new(),
+            global_buffer,
             local_buffer,
             extent: [config.width, config.height],
             rng,
@@ -370,7 +386,29 @@ impl crate::framework::Example for Example {
         _device: &wgpu::Device,
         _queue: &wgpu::Queue,
     ) {
-        //empty
+        let prior_vertical_extent = self.extent[1] as f32;
+        self.extent[0] = _sc_desc.width;
+        self.extent[1] = _sc_desc.height;
+        let globals = Globals {
+            mvp: glam::Mat4::orthographic_rh(
+                0.0,
+                _sc_desc.width as f32,
+                0.0,
+                _sc_desc.height as f32,
+                -1.0,
+                1.0,
+            )
+            .to_cols_array_2d(),
+            size: [BUNNY_SIZE; 2],
+            pad: [0.0; 2],
+        };
+        _queue.write_buffer(&self.global_buffer, 0, unsafe {
+            std::slice::from_raw_parts(
+                bytemuck::bytes_of(&globals).as_ptr() as *const u8,
+                80,
+            )
+        });
+        self.update_bunnies(prior_vertical_extent);
     }

     fn render(&mut self, view: &wgpu::TextureView, device: &wgpu::Device, queue: &wgpu::Queue) {

Another way to illustrate the lack of handling resize is with the below video. I imagine the wgpu logo should not appear stretched out as it does in the video:

https://github.com/user-attachments/assets/1f3e40aa-7dc1-4b19-950e-38ab3d2ee2f8

With the above patch, bunnymark is rendering correctly on Safari from the macOS 15 betas as well. It would be nice to handle resize so it doesn't appear broken in Safari:

bunnymark_safari

The reason it doesn't work in Safari currently is we report a 2x2 canvas on page load. Perhaps that is a separate issue, but it seems like handling resize would be a reasonable change.

dv29 commented 1 month ago

I wont be able to test this one as I don't have access to a Mac machine and google doesn't support nighly/canary on linux image

mwyrzykowski commented 1 month ago

@dv29 it shouldn't require a Mac to test. It also reproduces on Chrome stable (Version 126.0.6478.183 (Official Build) (arm64)).

As long as you can resize the window, the issue should reproduce. It can also be observed that the orthographic matrix is only computed on page load currently.

dv29 commented 1 month ago

Just tried on

OS:
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.4 LTS
Release:    22.04
Codename:   jammy

Google Chrome: Version 126.0.6478.182 (Official Build) (64-bit)

Screencast from 07-22-2024 08:05:25 PM.webm

mwyrzykowski commented 1 month ago

Doesn’t that also illustrate the issue? It seems odd the size of the bunnies is dependent on the initial window size.

In the current version, you see a difference if the page is loaded into a window of size 2000 x 300 and then resized to 1200 x 1200 compared to having the window sized to 1200 x 1200 initially.

it is because the orthographic matrix is not computed on resize.

dv29 commented 1 month ago

I opened the PR for some initial feedback. I got the vertical out of bounds covered though it still happens when the screen is resized. When the screen is resized it would have to recreate the global_group unless there is a way to just get the texture out of it and resize the texture, I'm not sure. @Wumpf any idea or should i go ahead and extract out global_group init code and recreate it when the screen changes?

Wumpf commented 1 month ago

not worth the hassle really, I'd just recreate the group as-is every time the screen resizes

dv29 commented 1 month ago

Updated the PR to recreate the global_group on resize #6034