Rename Density Volume to Local Volumetric Fog #3815
Merged
Conversation
This file contains hidden or 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
...ty.render-pipelines.high-definition/Editor/Lighting/VolumetricLighting/VolumetricMenuItem.cs
Outdated
Show resolved
Hide resolved
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightLoop.cs
Outdated
Show resolved
Hide resolved
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/lightlistbuild.compute
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks for this PR appreciated.
However I was guessing we were agree that we need to rename the C# class, to not create a discrepancy between display and script code. With all the migration problem issue, what is the status regarding this? Can we explore is there is a way to rename the class and perform a migration? thanks
jenniferd-unity
suggested changes
Mar 9, 2021
com.unity.render-pipelines.high-definition/Runtime/Debug/DebugDisplay.cs
Outdated
Show resolved
Hide resolved
| | **Size** | Controls the dimensions of the Volume. | | ||
| | **Per Axis Control** | Enable this to control blend distance per axis instead of globally. | | ||
| | **Blend Distance** | Blend Distance creates a fade from the fog level in the Volume to the fog level outside it. This is not a percentage, it is the absolute distance from the edge of the Volume bounds, defined by the Size property, where the fade starts. Unity clamps this value between 0 and half of the lowest axis value in the Size property. If you use the Normal tab, you can alter a single float value named Blend Distance, which gives a uniform fade in every direction. If you open the Advanced tab, you can use two fades per axis, one for each direction. For example, on the x-axis you could have one for left-to-right and one for right-to-left. Setting the distance to 0 hides the fade, while setting the distance to 1 creates a fade. | | ||
| | **Falloff Mode** | Controls the falloff function applied to the blending of **Blend Distance**. By default the falloff is linear but you can change it to exponential for a more realistic look. | |
There was a problem hiding this comment.
the image also needs updating
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightLoop.cs.hlsl
Show resolved
Hide resolved
|
The current stuff looks good although this needs an entry in the what's new page to explain the change. Also, if there is anything the user needs to do to upgrade their project (changed class name etc...), we need an entry in the relevant upgrading page. Also, as Jenny pointed out, the screenshots need updating too. Could you add these and ping me for a review?
Thanks! |
- Deprecating old stuff that must be set the Obsolete flag to true on 21.2
sebastienlagarde
approved these changes
Mar 11, 2021
|
oh forget to mention, please add a sectionin upgrade guide to mention this change, thanks |
alex-vazquez
changed the title
alex-vazquez
requested review from
JordanL8,
JulienIgnace-Unity,
RSlysz and
jenniferd-unity
and removed request for
JordanL8
RSlysz
requested changes
Mar 12, 2021
...unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/GlobalLightLoopSettings.cs
Show resolved
Hide resolved
RSlysz
approved these changes
Mar 15, 2021
jenniferd-unity
approved these changes
Mar 15, 2021
| { | ||
| [MenuItem("GameObject/Volume/Density Volume", priority = CoreUtils.Sections.section2 + CoreUtils.Priorities.gameObjectMenuPriority + 2)] | ||
| static void CreateDensityVolumeGameObject(MenuCommand menuCommand) | ||
| [MenuItem("GameObject/Local Volumetric Fog", priority = CoreUtils.Priorities.gameObjectMenuPriority + 2)] |
There was a problem hiding this comment.
@TheoWong-pixel could you confirm?
com.unity.render-pipelines.high-definition/Runtime/Debug/DebugDisplay.cs
Show resolved
Hide resolved
alex-vazquez
commented
Mar 16, 2021
com.unity.render-pipelines.high-definition/Runtime/Lighting/LightLoop/LightLoop.cs
Show resolved
Hide resolved
JulienIgnace-Unity
approved these changes
Mar 16, 2021
TomasKiniulis
suggested changes
Mar 16, 2021
There was a problem hiding this comment.
Thanks for the detailed description Alex! Updating template from 2020.3.1f1 to PR with additional various density volumes added seems to work correctly for me.
Listed some minor findings related to docs. Some sections do not sound grammatically correct where "Density Volumes" are replaced with "Local Volumetric Fog". @JordanL8 could you confirm if suggested changes are correct?
|
|
||
| - Density Volumes do not support volumetric shadowing. If you place a Density Volume between a Light and a surface, the Volume does not decrease the intensity of light that reaches the surface. | ||
| - Density Volumes are voxelized at a very coarse rate, with typically only 64 or 128 slices along the camera's focal axis. This can cause noticeable aliasing at the boundary of the Volume. You can hide the aliasing by using Density Volumes in conjunction with some global fog, if possible. You can also use a Density Mask and a non-zero Blend Distance to decrease the hardness of the edge. | ||
| - Local Volumetric Fog do not support volumetric shadowing. If you place a Local Volumetric Fog between a Light and a surface, the Volume does not decrease the intensity of light that reaches the surface. |
There was a problem hiding this comment.
Local Volumetric Fog do not support volumetric shadowing
Local Volumetric Fog does not support volumetric shadowing.
| - Density Volumes do not support volumetric shadowing. If you place a Density Volume between a Light and a surface, the Volume does not decrease the intensity of light that reaches the surface. | ||
| - Density Volumes are voxelized at a very coarse rate, with typically only 64 or 128 slices along the camera's focal axis. This can cause noticeable aliasing at the boundary of the Volume. You can hide the aliasing by using Density Volumes in conjunction with some global fog, if possible. You can also use a Density Mask and a non-zero Blend Distance to decrease the hardness of the edge. | ||
| - Local Volumetric Fog do not support volumetric shadowing. If you place a Local Volumetric Fog between a Light and a surface, the Volume does not decrease the intensity of light that reaches the surface. | ||
| - Local Volumetric Fog are voxelized at a very coarse rate, with typically only 64 or 128 slices along the camera's focal axis. This can cause noticeable aliasing at the boundary of the Volume. You can hide the aliasing by using Local Volumetric Fog in conjunction with some global fog, if possible. You can also use a Density Mask and a non-zero Blend Distance to decrease the hardness of the edge. |
There was a problem hiding this comment.
Local Volumetric Fog are voxelized at a very coarse rate
Local Volumetric Fog is voxelized at a very coarse rate
| | **Distance Fade End** | Distance from the camera at which the Density Volume has completely fade out. This is useful when optimizing a scene with many Density Volumes and making the more distant ones disappear| | ||
| | **Density Mask Texture** | Specifies a 3D texture mapped to the interior of the Volume. The Density Volume only uses the RGB channels of the texture for the fog color and A for the fog density multiplier. A value of 0 in the Texture alpha channel results in a Volume of 0 density, and the value of 1 results in the original constant (homogeneous) volume. | | ||
| | **Scroll Speed** | Specifies the speed (per-axis) at which the Density Volume scrolls the texture. If you set every axis to 0, the Density Volume does not scroll the texture and the fog is static. | | ||
| | **Distance Fade Start** | Distance from the camera at which the Local Volumetric Fog starts to fade out. This is useful when optimizing a scene with many Local Volumetric Fog and making the more distant ones disappear| |
There was a problem hiding this comment.
This is useful when optimizing a scene with many Local Volumetric Fog and making the more distant ones disappear
Should it be something like: "This is useful when optimizing a scene with lots of Local Volumetric Fogs and making the more distant ones disappear"?
| | **Density Mask Texture** | Specifies a 3D texture mapped to the interior of the Volume. The Density Volume only uses the RGB channels of the texture for the fog color and A for the fog density multiplier. A value of 0 in the Texture alpha channel results in a Volume of 0 density, and the value of 1 results in the original constant (homogeneous) volume. | | ||
| | **Scroll Speed** | Specifies the speed (per-axis) at which the Density Volume scrolls the texture. If you set every axis to 0, the Density Volume does not scroll the texture and the fog is static. | | ||
| | **Distance Fade Start** | Distance from the camera at which the Local Volumetric Fog starts to fade out. This is useful when optimizing a scene with many Local Volumetric Fog and making the more distant ones disappear| | ||
| | **Distance Fade End** | Distance from the camera at which the Local Volumetric Fog has completely fade out. This is useful when optimizing a scene with many Local Volumetric Fog and making the more distant ones disappear| |
There was a problem hiding this comment.
This is useful when optimizing a scene with many Local Volumetric Fog and making the more distant ones disappear
Maybe it should be "This is useful when optimizing a scene with lots of Local Volumetric Fogs and making the more distant ones disappear"?
| The **Max Local Volumetric Fog Size** controls the maximum resolution you can use for the **Density Mask Texture**. Changing this setting can have a large impact on memory usage. For information on how much memory HDRP allocates for the Local Volumetric Fog system, see the info box below this setting in the HDRP Asset. | ||
|
|
||
| The **Max Density Volume On Screen** controls how many Density Volumes can appear on-screen at once. This setting also has an impact on memory. | ||
| The **Max Local Volumetric Fog On Screen** controls how many Local Volumetric Fog can appear on-screen at once. This setting also has an impact on memory. |
There was a problem hiding this comment.
The Max Local Volumetric Fog On Screen controls how much Local Volumetric Fog can appear on-screen at once. This setting also has an impact on memory.
TomasKiniulis
approved these changes
Mar 16, 2021
alex-vazquez
added
ready-to-merge
…into x-pipeline/RenameDensityVolume
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
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.
Purpose of this PR
Rename Density Volume to Local Volumetric Fog
https://jira.unity3d.com/browse/XPIPELINE-90
Many of the changes done on this #2751, had been directly renamed as everything will go on the same version.
I've improved the Volumetric section on the HDRP Pipeline Asset Inspector, checked with @TheoWong-pixel
From this:
To this:
Deprecating old Density Volume public enums, clases, and methods all in a single new File
Deprecated.csas URP is doing.Added support for
InspectorNameAttributefor enumerations that will be added to the Render Pipeline Debugger WindowTesting status
Old Version:
New Version
Open the HDRP template and see that no errors are shown for density volumes
Check that if a new version uses the old API, there is a warning
Check the API Updater correctly changes the class names. Go to previous package 2021.1 and save a Density Volume, and script that obtains the component and the parameters. See that my new version the API Updater properly updates everything.
Run HDRP unit test locally