godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.08k stars 69 forks source link

Change Directional Shadow Splits from Percentage of Max Distance to Distance #4512

Open mrjustaguy opened 2 years ago

mrjustaguy commented 2 years ago

Describe the project you are working on

Tweaking Directional Shadows

Describe the problem or limitation you are having in your project

Splits are defined by how far away from the camera the shadows transition from one resolution to the other. For this reason it makes little sense that splits are calculated relatively to Max Distance, as this just causes several issues: 1) Unintuitive to Users 2) Loss of Split Precision at High Max Distances (due to having only 3 decimals to work with) 3) Changing Max Distance Changes all the Split Distances, something that can often be undesired, especially for the closer splits, as they are usually the first to be determined and locked in.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Instead of Splits being represented as percentages of Max Distance, Represent them for what they actually are: Distance of splits from the Camera.

1) More clear to Users, Especially New Users 2) You can Define a Split to be at exactly X units away even when you're dealing with a "Max Distance" 3) You can change the Distance of the Shadow without affecting the Split Distances

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Rename the following: Split 1 (in Percentage of MD) -> Distance to Split 1 (in Units) Split 2 (in Percentage of MD) -> Distance to Split 2 (in Units) Split 3 (in Percentage of MD) -> Distance to Split 3 (in Units) Max Distance (In Units) -> Distance to Split 4 (in Units) Fade Start (in Percentage of MD) -> Distance to Fade Start (If set to 0 disable fade)

Decouple Split distances from Max Distance and instead use the User Provided Split Distance Values Calculate order of Splits based on distances (for an edge case where the splits aren't 1<2<3<4, but for example 3<2<4<1, as all that matters is what the split distances are, and not that they are input in the correct order)

If this enhancement will not be used often, can it be worked around with a few lines of script?

Maybe

Is there a reason why this should be core and not an add-on in the asset library?

this is core UX.

Calinou commented 2 years ago

I'm not sure if this is a good idea, as this makes adjustments of Max Distance harder to perform quickly. If you keep the default split distances but increase Max Distance, close-up shadows will remain identical, but the last split will potentially become much lower-resolution. Even with https://github.com/godotengine/godot/pull/48776, this can introduce an ugly transition between the 3rd and 4th split (or the 1st and 2nd split when using PSSM 2 Splits).

Does any other engine use a similar method to adjust shadow split distances?

Loss of Split Precision at High Max Distances (due to having only 3 decimals to work with)

We can resolve this by modifying the property hints to allow for greater precision.

mrjustaguy commented 2 years ago

I'm not sure if this is a good idea, as this makes adjustments of Max Distance harder to perform quickly. If you keep the default split distances but increase Max Distance, close-up shadows will remain identical, but the last split will potentially become much lower-resolution. Even with godotengine/godot#48776, this can introduce an ugly transition between the 3rd and 4th split (or the 1st and 2nd split when using PSSM 2 Splits).

Technically this is a null point for 3 reasons: 1) unless you increase the resolution, the shadows will become un-usable for any complex meshes with the Max Distance change, forcing you to redo all the splits anyhow because all the splits moved 2) how visible the transition is also depends a ton on how far the actual transition is happening from camera, something that Max distance also changes currently, so poor use of Max Distance can also result in visible transitions appearing 3) having to redo splits can produce visible transitions appearing in both current and proposed, but this is much harder to do when you have to potentially drastically change all 4 splits due to a large change in max distance

I don't know if any other engines do splits this way...

There are Three main reasons why this Proposal was made: 1) More Efficient Workflows enabled 2) No need to do a ton of head math to figure out which split is where as it is explicitly provided by the user 3) Conceptually Simpler to understand, as you're essentially just working with a handful of independent controls, instead of a handful of connected controls

The workflow that this proposal makes really easy is as follows: Determine Resolution you are satisfied with Split 1 Determine Resolution you need to keep Split 2 Transition hidden Determine Resolution you need to keep Split 3 Transition hidden Determine Resolution you need to keep Split 4 Transition hidden Slightly Tweak Values according to your Shadow Distance (aka Split 4) needs

Currently, the workflow for that would look like this: Determine Max Distance you Think you'll need Determine Resolution you are satisfied with Split 1 Determine Resolution you need to keep Split 2 Transition hidden Determine Resolution you need to keep Split 3 Transition hidden Tweaking Max Distance forces changes to all 3 splits, however if you're only tweaking the 3 splits it's same as the workflow above

The hints would need 5, possibly even 6 decimals to get sufficiently accurate to have sufficiently fine grain control over the split distances, and that's messy.

In any case this is probably best suited to be in 4.0 if implemented, as this is going to be major compatibility breakage

WickedInsignia commented 9 months ago

Agreeing with @Calinou here, this could make Max Distance more difficult to use. The Max Distance cutoff and fade is usually of higher visual priority than where any of the individual splits will be. These changes feel like more of a personal workflow preference than a globally and objectively better one. Those renames also introduce interface clutter where Godot prioritizes compact layouts. Once the user knows that "split 1" really means "distance to split 1" there's no need to remind them every time they see it.

I think using metric distances for the splits makes sense though. To make changing splits faster it'd probably be best that we opt for some visual feedback instead, like the coloured radial fields Unity uses. These could show up automatically or when a modifier key is held while adjusting the split sliders. Unity Shadow Splits

Calinou commented 9 months ago

To make changing splits faster it'd probably be best that we opt for some visual feedback instead, like the coloured radial fields Unity uses. These could show up automatically or when a modifier key is held while adjusting the split sliders.

This is what https://github.com/godotengine/godot-proposals/issues/6097 proposes :slightly_smiling_face: