godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.93k stars 21.06k forks source link

VIEWPORT_SIZE uniform does not match the render target size while rendering to ReflectionProbe #80670

Open jamie-pate opened 1 year ago

jamie-pate commented 1 year ago

Godot version

3.5.2 Stable, 4.1.1 Stable

System information

Godot v4.1.1.stable - Ubuntu 20.04.6 LTS (Focal Fossa) - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3050 Laptop GPU () - 12th Gen Intel(R) Core(TM) i7-12700H (14 Threads)

Issue description

https://github.com/godotengine/godot/blob/3.5/drivers/gles3/rasterizer_scene_gles3.cpp#L4153 sets the VIEWPORT_SIZE uniform to the size of the viewport, but there is no way to get the size of the render target when rendering a ReflectionProbe

Steps to reproduce

Create a reflection probe, then use the VIEWPORT_SIZE in a shader that relies on the VIEWPORT_SIZE to generate the POINT_SIZE based on the Viewport (render target) resolution. In reflection probe textures the VIEWPORT_SIZE does not match the render target's resolution so the POINT_SIZE is wrong.

See attached minimal reproduction for godot3 and godot4

The minimal reproduction has 3 'Subject' meshinstances with PointMesh meshes and a shader that uses the VIEWPORT_SIZE to calculate the POINT_SIZE so that when the camera moves a way the 'voxel' gets smaller.

4.1.1 Vulkan (Forward+) Screenshot from 2023-08-15 16-22-37 3.5.2 GLES3 Screenshot from 2023-08-15 16-23-12

If you look at the reflections in the minimal reproduction the voxels from the reflection probe texture are overlapping and you can't see the middle one because they use the wrong VIEWPORT_SIZE (assuming that it is equivelant to the render target size)

Minimal reproduction project

shader-viewport-size-uniform-refprobe.zip

jamie-pate commented 1 year ago

viewport_size is set here for 3.5.2 https://github.com/godotengine/godot/blob/3.5/drivers/gles3/rasterizer_scene_gles3.cpp#L4153

Would developers expect the VIEWPORT_SIZE to match the render target size during ReflectionProbe texture rendering or would they expect it to match the current viewport size? Would it make sense to add RENDER_TARGET_SIZE as well? Or just set VIEWPORT_SIZE to match the size of the render target when doing the refprobe texture render?

jamie-pate commented 1 year ago

This issue makes my reflections look really bad :( Screenshot from 2023-08-15 16-34-52 Screenshot from 2023-08-15 16-34-35 image(1)

lawnjelly commented 1 year ago

This bit of code is not running during the reflection probes, because storage->frame.current_rt is NULL (I think set to NULL after drawing the viewports), and so I guess it is using the last set value for the viewport_size.

    if (storage->frame.current_rt) {
        int viewport_width_pixels = storage->frame.current_rt->width;
        int viewport_height_pixels = storage->frame.current_rt->height;

        state.ubo_data.viewport_size[0] = viewport_width_pixels;
        state.ubo_data.viewport_size[1] = viewport_height_pixels;

        state.ubo_data.screen_pixel_size[0] = 1.0 / viewport_width_pixels;
        state.ubo_data.screen_pixel_size[1] = 1.0 / viewport_height_pixels;
    }