godotengine / godot

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

Vulkan: SDFGI reflections appear to jitter when the camera rotates slowly #74633

Closed WickedInsignia closed 10 months ago

WickedInsignia commented 1 year ago

Godot version

4.0 Stable

System information

Windows 11, Nvidia RTX4070ti, AMD Ryzen7700x

Issue description

SDFGI reflections will visibly jitter with camera rotation, especially on materials with a metallic property of 1 and roughness property below 0.5. Will still occur on non-metallic materials. This is visible at runtime as well as in the editor. It is not visible when panning/moving the camera laterally.

Steps to reproduce

Apply a reflective material to any object and observe the effect while the camera is being moved to "look around" with the mouse. Alternatively, load MPR and observe reflective cubes while moving camera around.

Minimal reproduction project

SDFGI Reflection Jitter.zip

Calinou commented 1 year ago

This is due to limited precision from the algorithm used to calculate reflections (for performance reasons). I think the SDFGI rework will address this in some way, but I'm not sure how.

WickedInsignia commented 1 year ago

SDFGI is flickering like wild on this particular mesh, for whatever reason. The whole interior is jittering with camera movement. Thought it might be helpful to attach the most severe case I've seen.

Godot_SFGIJitter

LightmapLeakingV2.zip

krazyjakee commented 1 year ago

As I see it, this bug means reflections are unusable in a project where the player has control of the camera. I dug in and got some explanation from devs and ai.

Simplified Explanation: The Godot engine uses something called a "normal buffer" as part of its rendering process. A buffer is basically a space in memory that stores data temporarily while it's being moved or used. In the context of 3D graphics, a normal buffer specifically holds information about the direction surfaces are facing in the 3D space. This information is crucial for accurately calculating how light should bounce off objects, which in turn affects things like reflections and lighting.

Now, the issue you're facing with the jittery reflections is likely due to the precision of this normal buffer. "Precision" refers to the detail level that the buffer can handle—think of it as the difference between drawing with a fine pen versus a chunky marker. When the precision is low, the engine can't accurately track the exact directions of surfaces, leading to errors in reflections, especially when the camera moves or rotates.

One proposed solution is to use "octahedron mapping" for the normals instead of the current method. Octahedron mapping is a way of storing and processing normal vectors that can reduce errors and improve the precision without needing more memory. It basically allows for a more detailed representation of how surfaces are oriented by wrapping them around an octahedron shape.

However, switching to octahedron mapping isn't straightforward because the normal buffer's data is directly available to users—who might be using this data for their own custom shaders or effects. Changing the format of this buffer would affect everyone who relies on the current system, so it's not a simple switch.

Couldn't user shaders simply recalculate the normal under octahedron mapping?

Yes, it is theoretically possible for a shader to recalculate the normal under octahedron mapping on the fly, although this would be an advanced technique and would involve additional computation that could impact performance, especially on less powerful hardware.

Here's how you could think about implementing this in a shader:

While this process can potentially reduce the artifacts caused by low-precision normals, it requires that the shader do quite a bit of extra math. Shaders operate on the GPU and are run per-pixel for fragments or per-vertex for vertices, so this could add up to a significant amount of extra calculations, especially for complex scenes or high-resolution outputs.

WickedInsignia commented 1 year ago

@krazyjakee current-gen AI is known to hallucinate information or be simply unfactual, and it can be very difficult for a layman to distinguish what is true and what is not. I'd strongly suggest refraining from its usage in technical discussions where firsthand knowledge proves the most reliable and useful, or at least disclosing exactly what information is derived from AI and fully validating its claims.

krazyjakee commented 1 year ago

Firstly, I was getting confused, uninformative one-liners from the people with "first hand" knowledge. When asking 2 questions they would only reply to the first then ignore me etc. It's far more likely they would correct wrong information than provide an answer to a question, that's just human nature.

Second, there is nothing wrong with the information above. It has been heavily doctored by me based on my new understanding of octahedron mapping, its purpose and implications for existing functionality and user land implementations.

WickedInsignia commented 1 year ago

Unfortunately I don’t believe utilising AI to support your newfound knowledge is going to net you any more attention here than those with first-hand knowledge offered you.

If you believe you understand the topic enough to implement a fix then definitely feel free to take a crack at it! This is an open-source engine with a problem wide open and ready for contribution. Your claims above would find validation in the case that it’s successful and the effort would be greatly appreciated.

Otherwise please understand why myself (and likely others) take the appraisal above with a grain of salt.

krazyjakee commented 1 year ago

@WickedInsignia It's embarrassing. Do you not think it's a bit sad that after 8 months of silence on this subject, somebody comes along, talks to the core team, looks up different solutions and works with AI to help explain some of the higher level aspects of this. They do a write up for others benefit and the first thing you do is to shut this person down because "AI bad".

We all know some of the Godot goons suffer from these inherent ego issues but when it comes to contributions, isn't that your bread and butter? Shouldn't you be embracing those who try and help? I urge you to wait for someone to come along and approve my write-up before dismissing it on such very trivial grounds.

ThIs Is An OpEn SoUrCe EnGiNe

Good lord. Why do you think I am here try to spark discussions on the correct approach? Trying to reach the core team on this subject? The reasons why I don't dive in and open a PR are explained in the write up above. Did you even bother to read it? An open source engine project also mean YOU can leave any time.

WickedInsignia commented 1 year ago

This is not an attempt to shut you down nor do I have any authority to do so. I'm simply exercising caution around your appraisal because AI was used and you mentioned it was newfound knowledge on your part. I'm no expert on the subject and have chosen to await a second opinion on the viability of that solution. That is my own prerogative. You're perfectly welcome to contribute to resolve the issue and nothing I say here is designed to prevent that. I reported a bug, my interests reside in it being resolved.

clayjohn commented 1 year ago

Okay, time to end this conversation. It's going nowhere.

Also worth noting, calling Godot devs "goons" is a violation of our code of conduct and is far below the expectations we have for people engaging in the community. Please be kinder in the future.

clayjohn commented 12 months ago

Following up on this. I did a quick test using octahedral encoded normals to see if it improves the situation in the MRP and it does not. Accordingly, the source of the jitter is not low-precision normals, it is something else that needs to be investigated.

WickedInsignia commented 12 months ago

This issue and #84746 pose the biggest hurdles to VoxelGI's viability in production in my tests. Everything else is rather small but these are the show-stoppers for many use cases. Thanks for investigating this! I'll make sure to test any developments.