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

Add double precision support for World Triplanar Mapping #91380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fire
Copy link
Member

@fire fire commented Apr 30, 2024

Salvages #75577

image

The conflicts are mostly on these lines in the shader.

Use https://github.com/V-Sekai/godot-double-precision-test

Bugsquad edit:

@fire fire requested review from a team as code owners April 30, 2024 22:41
@fire fire force-pushed the implement_double_precision_for_world_triplanar branch from 1494ab2 to 422f514 Compare April 30, 2024 22:45
@fire
Copy link
Member Author

fire commented Apr 30, 2024

The center which is the triplanar material is correct in both far and near.

Screen.Recording.2024-04-30.at.3.47.50.PM.mov

@shakesoda
Copy link

tested working here, thank you

@fire fire force-pushed the implement_double_precision_for_world_triplanar branch 2 times, most recently from 25e78bf to ae60904 Compare June 23, 2024 15:46
@fire
Copy link
Member Author

fire commented Jun 23, 2024

Resolved a conflict in servers/rendering/shader_types.cpp where upstream changed some of the variable locations.

@fire fire force-pushed the implement_double_precision_for_world_triplanar branch 3 times, most recently from fe5a9d0 to aa45100 Compare June 26, 2024 00:59
@fire
Copy link
Member Author

fire commented Jun 26, 2024

Updated to 4.3-beta2.

This commit adds emulated doubles support for the World UV Triplanar Mapping.
It also adds some QoL features to both ease the "integration" with the double
precision pipeline and also to allow greater flexibility when dealing
with Triplanar UVs.

Features:
  - New mat4 TRIPLANAR_MATRIX vertex-only property (the same as MODEL_MATRIX by default).
  - New "hint_triplanar_mat" hint for mat4 uniforms, which allow users
    to specify a custom TRIPLANAR_MATRIX. Naturally, only a single
    instance of the hint is allowed per shader.
  - New vec3 TRIPLANAR_POSITION vertex-only property, which holds the
    VERTEX transformed by the TRIPLANAR_MATRIX (With emulated doubles, if
    applicable).
  - Updated Standard 3D Material to make use of the above features.
  - Updated VisualShader to include the TRIPLANAR_MATRIX and
    TRIPLANAR_POSITION as valid Inputs.
  - Added a "hint_triplanar_enabled" toggle to the VisualShaderNodeTransformParameter to allow
    the new "hint_triplanar_mat" hint to be set.

Note: The shader related changes were applied to all renderers (Forward+,
Mobile and Compatibility).

Note²: Given the lack of emulated doubles support for the Compatibility
renderer, only the single precision path was implemented.

Co-Authored-By: =?UTF-8?q?Jo=C3=A3o=20Pedro=20Braz?= <brazjoaopedro@ymail.com>
@fire fire force-pushed the implement_double_precision_for_world_triplanar branch from aa45100 to a9d22cd Compare August 21, 2024 15:16
@fire
Copy link
Member Author

fire commented Aug 21, 2024

Updated to master. Had a code conflict in servers.

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.

I'm not so sure about this PR. Its seems way too hacky to me. It is making huge changes in our shader compiler and internal shader to support something in the standard material.

Also, It looks like the new hint isn't even used in the StandardMaterial3D? So won't it remain broken for everyone?

It also relies on users manually passing in another matrix as a uniform and then secretly inserting that matrix into the backend code.

I think its worthwhile to see if we can improve this behaviour with a much simpler approach

ALso keep in mind our best practices https://docs.godotengine.org/en/latest/contributing/development/best_practices_for_engine_contributors.html#prefer-local-solutions particularly "Prefer local solutions".

@fire
Copy link
Member Author

fire commented Aug 21, 2024

Will need a rethink then, was trying to salvage some older prs

@fire
Copy link
Member Author

fire commented Nov 9, 2024

I am not able to fix this at this moment, but the pr is still wanted and is salvageable.

@fire
Copy link
Member Author

fire commented Dec 10, 2024

I don't think I can salvage this pull request but it is still wanted.

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.

World Triplanar is broken with double-precision build
4 participants