2021.1 Universal/fix aces filter #3776
Conversation
…sion for Switch platform in order to fix ACES tonemapping artefact. This issue does not affect other mobile platforms.
|
It appears that you made a non-draft PR! |
kaychang-unity
changed the title
kaychang-unity
changed the title
| #if defined(SHADER_API_SWITCH) // Must use full float precision on Switch to avoid artefact when using ACES tonemapping | ||
| float3 LinearToLMS(float3 x) | ||
| #else | ||
| real3 LinearToLMS(real3 x) | ||
| #endif |
There was a problem hiding this comment.
Imo if this function needs full precision it should be changed for all platforms, otherwise we have the same issue on mobile. Checking the original PR, @eh-unity made the same comment.
There was a problem hiding this comment.
I tested on iPhone8 metal and Galaxy S9 Mali-G72 Vulkan but could not reproduce the issue, hence why it is conservatively kept Switch only. But I can enable it for all platforms if you prefer. There is similar floating point fix in AcesTonemap() and NeutralCurve() for Switch but done from a different PR, do you want to change them for all platforms too?
phi-lira
left a comment
phi-lira
left a comment
There was a problem hiding this comment.
LGTM. Are you going to also put a PR to master to change the behaviour to use float instead of half for all platforms?
Backport from master Purpose of #2880
This fixes ACES tonemapping not correctly working on Nintendo Switch.
The generated LUT had artefacts due to fp16 precision.
I tested Android Vulkan devices (Google Pixel 2, Glaxy S9 on ARM GPU), as well as iOS Metal (Iphone8), but the issue does not happens on these platforms.