LOD: Remove "Raycast Normals" and associated "Normal Split Angle" settings #98620
+9
−309
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tings "Raycast Normals" was introduced in 4.4 dev and defaulted to "false". The limited testing results at the time suggested that raycasting generally reduces normal quality compared to native simplifier results, at the same time increasing vertex memory and import time. To play it safe, we introduced a setting that defaulted to false, with the goal of removing it later in 4.4 development cycle if no regressions are noticed. Since we already had three dev snapshots and no reports, this change removes the setting and associated code. "Normal Split Angle" was only used when raycast normals were enabled; this change removes it from the settings, but keeps it in the script binding for compatibility. Existing meshes import exactly the same after this change (unless they chose to override raycasting which would be surprising). split_normals helper was only used in this code path and is also removed for simplicity; it is unlikely that this code will be useful as is, as it can only regenerate normals without fixing tangents or updating positions.
|
Mesh import settings after this change ("10" is the value I've set manually before this PR to check that it persists correctly, the default is still 60):
|
clayjohn
approved these changes
Oct 31, 2024
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
"Raycast Normals" was introduced in 4.4 dev and defaulted to "false". The limited testing results at the time suggested that raycasting generally reduces normal quality compared to native simplifier results, at the same time increasing vertex memory and import time.
To play it safe, we introduced a setting that defaulted to false, with the goal of removing it later in 4.4 development cycle if no regressions are noticed. Since we already had three dev snapshots and no reports, this change removes the setting and associated code.
"Normal Split Angle" was only used when raycast normals were enabled; this change removes it from the settings, but keeps it in the script binding for compatibility.
split_normalshelper was only used in this code path and is also removed for simplicity; it is unlikely that this code will be useful as is, as it can only regenerate normals without fixing tangents or updating positions.Note that documentation was updated to not mention the "suggested" values for merge normals angle in the script binding. The recommendation given by the documentation was reversed from the default (which is 60 and 25, not 25 and 60!), and 60 seems too aggressive to recommend even if that's the current import default.
Tested by importing existing projects (both with default settings and with overridden merge angle per mesh); meshes import the same after this change. If a mesh had "raycast normals" setting enabled, it will now import without it as the setting was removed; this is the same as the transition from any 4.3 project to earlier 4.4 dev builds so no new behavior is introduced unless raycast normals were enabled in 4.4 dev which would be unusual.