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

Use MODELVIEW_MATRIX when on double precision #75462

Conversation

joao-pedro-braz
Copy link
Contributor

Description

This commit updates the double precision vertex transform code to use the MODELVIEW_MATRIX instead of the MODEL_MATRIX.

This is done by transforming the MODELVIEW_MATRIX back into model space and then using it as if it were the MODEL_MATRIX.

With this in place we now properly handle VERTEX transformations that a Material Shader might do, such as billboard-ing.
This also fixes #75433.

Comparisons

(All done using godotengine/godot-demo-projects#894)

With this fix + double precision
https://user-images.githubusercontent.com/37230465/228559865-947ef102-f430-45b4-b437-7b3ebc8b16a4.mp4
It does jitter quite a bit on the farthest setting, but that happens regardless of this fix, so it doesn't seem like a regression.

Without this fix + double precision
https://user-images.githubusercontent.com/37230465/228562053-12cbea3c-2387-4985-b39e-25959011434d.mp4
Same jittering, but effects like billboarding stops working.

…abled

This commit updates the double precision vertex transform code from
using the MODEL_MATRIX to now use the MODELVIEW_MATRIX instead.

This can be made possible by transforming the MODELVIEW_MATRIX
back into model space (ie, same space as the MODEL_MATRIX) and then using it as if it were the MODEL_MATRIX.

With this in place we now properly handle VERTEX transformations that
a Material Shader might do, such as billboard-ing.
@Zylann
Copy link
Contributor

Zylann commented Mar 31, 2023

Maybe could be tested with the scene from this post too, which also contains decals, animated meshes, multimeshes and triplanar mapped materials #58516 (no time to do this test right now)

@clayjohn
Copy link
Member

I have a feeling that this is essentially forcing single precision. Does it still work fine when you are several thousands of units away from the world origin?

@joao-pedro-braz
Copy link
Contributor Author

joao-pedro-braz commented Mar 31, 2023

I have a feeling that this is essentially forcing single precision. Does it still work fine when you are several thousands of units away from the world origin?

It does.

On the first demo (https://user-images.githubusercontent.com/37230465/228559865-947ef102-f430-45b4-b437-7b3ebc8b16a4.mp4) you can see at 0:15 that I move 10_000_000_000_000 units away from the origin on the X axis and everything continues to work as expected.
@clayjohn

@joao-pedro-braz
Copy link
Contributor Author

Maybe could be tested with the scene from this post too, which also contains decals, animated meshes, multimeshes and triplanar mapped materials #58516 (no time to do this test right now)

I'll test rn!
@Zylann

@joao-pedro-braz
Copy link
Contributor Author

joao-pedro-braz commented Mar 31, 2023

4.0 stable (single precision)
https://user-images.githubusercontent.com/37230465/228999237-c5c38bcb-5089-4973-bfc3-887c6e661234.mp4

4.1.dev (double precision, with this fix)
https://user-images.githubusercontent.com/37230465/228999324-ace30b99-1cdc-4e30-9278-48f41d7d9b1c.mp4

4.1.dev (double precision, without this fix)
https://user-images.githubusercontent.com/37230465/228999939-b0608814-bea8-4170-9371-a28b74366c17.mp4

TLDR (or rather TLDW lol): Single precision has a very noticeable flicker when far from the origin (as expected), which isn´t the case with neither of the double precision versions (with and without this fix).
World triplanar mapping is broken in all of them (when far from the origin, ofc).

@Zylann

@joao-pedro-braz
Copy link
Contributor Author

@clayjohn @Zylann

Managed to get World Triplanar Mapping working with emulated doubles!

I mentioned on Rocket.Chat an approach to fix that using some GDScript to compute a "clamped-uv-texture-sized world position", which does work, but requires some user effort to implement.

After giving it some more thought, I managed to get it working by essentially using a origin shifted VERTEX when computing the UV World Position. That way we still have our UVs sampled in World space, but with the full range of precision that a float between 0 and 1 has.

scene shader:
image
(Gonna refactor this later, just a PoC)

material:
image

As you can see, I introduced a new builtin called "VERTEX_POSITION_WORLD", which is used when computing the UV Triplanar Position.
I know that the name is sort of misleading, given that on double precision it doesn't really represents the VERTEX in world space, I do plan to fix that before trying to merge it.

As for the results themselves:

With the fix:

Screenshot:
image
image
(At 10_000_000_000_000 units away from the origin it does start to break, but at that point I guess it's kinda of expected?)

Video Demo:
https://user-images.githubusercontent.com/37230465/229194547-10bed7d2-584d-4d21-8d75-3b56699fc24c.mp4

Without the fix:

Screenshot:
image

Video Demo:
https://user-images.githubusercontent.com/37230465/229195683-6a616ccd-d8ad-488d-ad96-e8ba9934a419.mp4

Would love to hear some feedback!
Also, is it okay if I add this as part of this PR or do you rather I create another one?

@fire
Copy link
Member

fire commented Mar 31, 2023

I think it's fine, but smaller and separate commits tend to be reviewed faster in my experience.

@joao-pedro-braz
Copy link
Contributor Author

Glad to know that, I'll open another PR then.
The easier the merrier!

@Zylann
Copy link
Contributor

Zylann commented Mar 31, 2023

World triplanar was expectedly broken in this case, but it's nice you found a way to make it work. I wonder if it's possible to use the same fix when doing triplanar with custom shader code (although I use triplanar with an extra transform since things are moving, I don't use world directly).

@joao-pedro-braz
Copy link
Contributor Author

It should be possible, given that the Material Shader itself is using the 'VERTEX_POSITION_WORLD" to compute the UV pos.
Could you share your custom example? I could test it for ya

@Zylann
Copy link
Contributor

Zylann commented Mar 31, 2023

I dont have a ready-made minimal project where I use this. Here is an occurrence of it: https://github.com/Zylann/solar_system_demo/blob/bdbaf5edc622e0cd2cae2cb6fed078d29eb9abe6/solar_system/materials/planet_ground.gdshader#L105
Notice how the UVs come from a transform calculation from a matrix passed by uniform, instead of either world or model. Basically the idea is that I have moving objects that are composed of many different meshes, so the only way I can make triplanar look good on them is to pass a transform for the whole object instead of using world or model.

@joao-pedro-braz
Copy link
Contributor Author

Wow!
Funny enough I encountered your demo some days ago, even have it cloned locally, lol.
I'll have a look at it!

@joao-pedro-braz
Copy link
Contributor Author

joao-pedro-braz commented Apr 1, 2023

Created the PR for the World Triplanar Mapping stuff: #75577
This is one is ready to be reviewed and merged when applicable.

Also @Zylann I think I managed to solve your use case pretty well!
You can check the PR for a video demo where I showcase the new "hint_triplanar_mat", which allows us to do the Triplanar UV mapping using a custom mat4 uniform.
The gist is:

image
Shader

image
GDScript

Basically, the hint sets the TRIPLANAR_MATRIX value, which in turn is used to compute the TRIPLANAR_POSITION, using the emulated doubles if double precision is enabled.

@cridenour
Copy link
Contributor

Thanks for this. I've tested it locally and it works great.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some careful thought and testing this seems fine.

I am hesitant because it adds a couple extra matrix multiplies (if the modelview matrix is never used it gets compiled out, same with the read_model_matrix). However, this is a nice solution to the problem that requires no changes on the user's end. Users will understand that enabling double precision comes with a cost.

Initially I was concerned about losing precision, but this doesn't actually have a big impact on precision for two reasons:

  1. The origin of the model_matrix is thrown away even though it is used in the billboarding calculation, the original origin is still used
  2. The positions from particles are single precision anyway

One possibility for improvement would be to add a usage_define MODELVIEW_MATRIX_USED and then only use this hack when the modelview matrix is referenced in a user shader. This would save some performance when not billboarding.

@akien-mga akien-mga merged commit 34a842b into godotengine:master Apr 13, 2023
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

@YuriSizov
Copy link
Contributor

Cherry-picked for 4.0.3.

@akien-mga akien-mga changed the title Use MODELVIEW_MATRIX when on double precision Use MODELVIEW_MATRIX when on double precision May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Billboarding not working when building with precision=double
9 participants