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

Vulkan Mobile: Changing 3D scaling amount with Bilinear/FSR scaling modes creates multiple errors on Output console/Debugger #70093

Closed
wokeraccoon opened this issue Dec 15, 2022 · 12 comments · Fixed by #80368

Comments

@wokeraccoon
Copy link

wokeraccoon commented Dec 15, 2022


Bugsquad note: This issue has been confirmed several times already. No need to confirm it further.


Godot version

v4.0.beta8.official [45cac42]

System information

Fedora Linux 37, AMD RX 6600, AMD Ryzen 7 3700x

Issue description

When using the mobile renderer instead of the forward_plus renderer and then increasing or decreasing the scaling, the Output console print these error messages on what I'm assume is every frame:

All textures in a framebuffer should be the same size.
./servers/rendering/renderer_rd/effects/../storage_rd/../framebuffer_cache_rd.h:170 - Condition "rid.is_null()" is true. Returning: rid

image

When running an scene, the engine prints similar errors to the Debugger in the Errors tab:

image

Using forward_plus does not exhibit the same problems.

Steps to reproduce

  1. Create a new project with a 3D Scene with at least one camera
  2. Go to Project Settings, enable Advanced Settings and go to rendering/renderer/rendering_method and switch from forward_plus to mobile
  3. Go to rendering/scaling_3d/scale and increase or decrease the slider
  4. Watch the Output console overflow with the same two errors
  5. Run the scene
  6. Watch the Errors tab in the debugger overflow with the same errors

Minimal reproduction project

test-bug.zip

Open the project. It's already set up with a camera, lighting and a cube mesh and the renderer is already using the mobile renderer with the scaling set to 0.64. The console should be already outputting the same errors.

@clayjohn clayjohn added this to the 4.x milestone Dec 15, 2022
@Calinou Calinou changed the title Changing 3D scaling amount while using the mobile renderer with either Bilinear/FSR scaling modes creates multiple errors on Output console/Debugger Vulkan Mobile: Changing 3D scaling amount with Bilinear/FSR scaling modes creates multiple errors on Output console/Debugger Dec 15, 2022
@BastiaanOlij
Copy link
Contributor

I'll have a look at this next week, I've been wanting to dive back into this to implement that proposal I did a while ago to properly type things. There must be a place in the code where we're forgetting to resize a buffer.

@BastiaanOlij BastiaanOlij self-assigned this Dec 15, 2022
@lattalayta
Copy link

I am also getting this error when using the mobile renderer and adjusting the render3D scaling in Godot 4.0.2

@marius-se
Copy link
Contributor

Does this have any effects, besides console spam? I'm getting the same error messages.

@Bimbam360
Copy link

Chipping in to add the following error spam on the latest 4.1.dev when using a non 1.0 scaling amount:

E 0:00:04:0691 get_color_fbs: Condition "target_size != internal_size" is true. Returning: RID()
<C++ Source> servers/rendering/renderer_rd/forward_mobile/render_forward_mobile.cpp:256 @ get_color_fbs()

@clayjohn
Copy link
Member

clayjohn commented May 3, 2023

Looks like this issue was fixed for 4.0 and then re-introduced into 4.0.1 with #74150

But looking at the code, I can't understand why

CC @BastiaanOlij

@BastiaanOlij
Copy link
Contributor

Looks like a check went missing somewhere, I'll have to find some time to look into it. Just for background info for anyone wanting to look into this earlier:

We perform 4 basic passes (not counting certain post effects), Opaque, Sky, Transparent and tonemapping.
The mobile renderer attempts to handle all 4 passes as subpasses.
However subpasses require all actions through all passes to stay within a tile, and all the render targets have to be the same size.
If this is not possible the mobile renderer will perform the transparent and/or tonemapping passes as a normal pass.

So our options are:

  • All four passes as subpasses, this is the most optimal scenario for mobile GPUs.
  • Opaque, sky and transparent as subpasses, tonemapping as a normal pass
  • Opaque and sky as subpasses, transparent as a normal pass, tonemapping as a normal pass (this happens when you read from screen or depth textures as we introduce a buffer copy after sky and can read outside of our tile).

Tonemapping needs to run in it's own pass if we use DOF, Glow, auto exposure or when our 3d buffer is scaled.

The error @Bimbam360 is reporting happens when our logic decides to use only subpasses, but than runs into trouble when it turns out it can't because the destination buffer is the wrong size.

My guess without looking into the current code is that when 2D MSAA is used, we're not getting the right size information somewhere.

@Calinou
Copy link
Member

Calinou commented May 3, 2023

In 4.1.dev d6dde81, 2D MSAA also breaks 3D rendering in the Forward+ rendering method when using a 3D Scaling value different from 1.0.

@BastiaanOlij
Copy link
Contributor

I'll see if I can dive into this over the weekend, currently trying to get some momentum going with some of the other changes I'm working on :)

@Janblu
Copy link

Janblu commented Jul 8, 2023

I'm getting this error in 4.1.stable, setting the rendering/scaling_3d/scale to 1 and not changing it at runtime removes the error.

@kesocos
Copy link

kesocos commented Jul 15, 2023

I'm on 4.1 Stable and I'm still getting this problem. Moving my project to forward+ does remove the error spam.

@mahdisml
Copy link

This error is happening again in 4.1.1 Mobile Renderer (When i Change the Scale Value to 0.5) :

image

@Calinou
Copy link
Member

Calinou commented Sep 28, 2023

This error is happening again in 4.1.1 Mobile Renderer (When i Change the Scale Value to 0.5) :

The pull request hasn't been cherry-picked to 4.1 yet, and no 4.1.x release has been made with the pull request either. If you test 4.2.dev5, you should notice the issue is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.