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

Shadow map comparison sampler always produces shadow on WebGL2 #2138

Closed
superdump opened this issue Oct 31, 2021 · 9 comments
Closed

Shadow map comparison sampler always produces shadow on WebGL2 #2138

superdump opened this issue Oct 31, 2021 · 9 comments

Comments

@superdump
Copy link
Contributor

Description

In bevy we are reworking the renderer and have a fairly simple test scene we use for demonstrating basic 3D rendering with 3 boxes for walls, 3 spheres representing the positions of 3 coloured point lights (red, green, blue), and a cube and another sphere as just objects in the scene to be lit and cast shadows. There is also a directional light which is pointing toward the ground plane at a 45 degree angle, but rotating about the y axis over time.

Repro steps

See the attached zip file. It contains the wasm, js, html necessary to run the bevy wgpu WebGL2 version of this example application. Extract it, serve it using some local http server (I've been using the rust simple-http-server crate with --nocache to make sure it's loading fresh across rebuilds) and open it in your browser of choice that supports WebGL2.

Expected vs observed behavior

Expected result from a native build of the exact same bevy wgpu code:
Screenshot 2021-10-31 163459

Result from Firefox:
Screenshot 2021-10-31 163532
Note that there is no red light. The red light is the first of the three point lights and it should cast shadows. WebGL2 does not support cube array textures, so I have had to drop the code down to support a single point light shadow map. Also note that the white directional light is not casting any shadows.

Result from Chrome:
Screenshot 2021-10-31 165138
Note that the lighting looks very similar to the native version, but there are no shadows from the red light (which is presumably just never occluded in this case, compared to always occluded in the Firefox case) nor are there shadows from the white directional light.

Extra materials
Zip file of the web build of the code: web.zip Unpack it, serve it, open it.

Platform
OS: Windows 10
wgpu 0.11.0, naga 0.7.1, bevy running something very close to the pipelined-rendering branch which represents the current state of the renderer rework. Using the Vulkan backend for native. Mobile RTX 3080.

@kvark
Copy link
Member

kvark commented Nov 1, 2021

Thank you for filing! Love the details here, that's very helpful to resolve it.
After you reported the bug on matrix, you had an extensive conversation with @magcius on the issue. Was everything resolved, or are there still unknowns in this story?

@magcius
Copy link
Contributor

magcius commented Nov 1, 2021

The other issue is the directional shadow map caused by the array-of-size-1 thing.

@kvark
Copy link
Member

kvark commented Nov 1, 2021

Just for clarity, we have this problem with GL backend that it guesses the actual type (or bind point) of a texture based on the array layer count:

  • if it's positive and divisible by 6, we assume a cube map
  • if it's larger than 1, we assume an array
  • otherwise, a regular 2D texture

This is pretty crappy, to put it humbly.

@superdump
Copy link
Contributor Author

Thank you for filing! Love the details here, that's very helpful to resolve it. After you reported the bug on matrix, you had an extensive conversation with @magcius on the issue. Was everything resolved, or are there still unknowns in this story?

The PR that was merged (#2139) did not solve the cubemap rendering issue. However, @magcius 's first patch proposed on matrix (with a very small fix) does solve it:

diff --git a/wgpu-hal/src/gles/queue.rs b/wgpu-hal/src/gles/queue.rs
index 99ba0033..2344f658 100644
--- a/wgpu-hal/src/gles/queue.rs
+++ b/wgpu-hal/src/gles/queue.rs
@@ -20,6 +20,13 @@ fn extract_marker<'a>(data: &'a [u8], range: &std::ops::Range<u32>) -> &'a str {
     std::str::from_utf8(&data[range.start as usize..range.end as usize]).unwrap()
 }
 
+fn is_cubemap_target(target: super::BindTarget) -> bool {
+    match target {
+        glow::TEXTURE_CUBE_MAP | glow::TEXTURE_CUBE_MAP_ARRAY => true,
+        _ => false,
+    }
+}
+
 fn is_3d_target(target: super::BindTarget) -> bool {
     match target {
         glow::TEXTURE_2D_ARRAY | glow::TEXTURE_3D => true,
@@ -91,6 +98,14 @@ impl super::Queue {
                         view.mip_levels.start as i32,
                         view.array_layers.start as i32,
                     );
+                } else if is_cubemap_target(target) {
+                    gl.framebuffer_texture_2d(
+                        fbo_target,
+                        attachment,
+                        CUBEMAP_FACES[view.array_layers.start as i32],
+                        Some(raw),
+                        view.mip_levels.start as i32,
+                    );
                 } else {
                     gl.framebuffer_texture_2d(
                         fbo_target,

And for the directional shadow map, if I either use D2 instead of D2Array, or use more than 1 layer, it works. Unless this is solvable I guess we will have to work around it, and that can be fine.

So for me, the outstanding issue is that the cubemap fix PR that was merged doesn't fix the problem.

@kvark
Copy link
Member

kvark commented Nov 1, 2021

Is this still an issue with #2143?

@magcius
Copy link
Contributor

magcius commented Nov 1, 2021

The texture layer issue probably still applies, but perhaps we should file a separate bug for this.

@superdump
Copy link
Contributor Author

Is this still an issue with #2143?

That depends whether the interpretation of a 2d array texture in wgsl, with 1 layer, as a non-array and silently not working as a consequence is deemed ok or not. I can work around it but it’s very non-obvious as the code is perfectly valid for other backends. I defer that to you all as I just don’t know enough about GL to comment.

@superdump
Copy link
Contributor Author

I filed a new issue attempting to describe the array texture issue though I feel my fuzzy burnout brain maybe didn't do it so clearly. Hopefully it's good enough: #2161

@superdump
Copy link
Contributor Author

Closing as the remaining problem is in #2161 .

Patryk27 pushed a commit to Patryk27/wgpu that referenced this issue Jan 7, 2023
* Fix textureGather on texture_2d<u32/i32>

* Add textureGather u32/i32 tests

* Update src/valid/expression.rs

Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com>

* Fix formatting

* undo analyzer change

Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@users.noreply.github.com>
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

No branches or pull requests

3 participants