Skip to content

Add Posterize option to LTCGI Diffuse #85

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

Merged
merged 12 commits into from
May 25, 2025
Merged

Add Posterize option to LTCGI Diffuse #85

merged 12 commits into from
May 25, 2025

Conversation

fundale
Copy link

@fundale fundale commented Mar 3, 2025

No description provided.

@orels1
Copy link
Owner

orels1 commented Mar 4, 2025

Hmm, I'm not really sure I want to add a branch just for this 🤔
Especially in the LTCGI module that also gets used by world shaders.

The v2 of the toon shader (which is what I assume this is meant to be used with) could probably support an extra spot where you posterize the LTCGI results in its own module.

However, thinking about it some more it feels like doing something like using the Toon Ramp and other shading settings might be what you actually want? Then instead of just pure posterization with discrete steps - you could have full ramp-based control over how the diffuse shading from LTCGI is applied? 🤔

@fundale
Copy link
Author

fundale commented Mar 4, 2025

Hmm, I'm not really sure I want to add a branch just for this 🤔 Especially in the LTCGI module that also gets used by world shaders.

The v2 of the toon shader (which is what I assume this is meant to be used with) could probably support an extra spot where you posterize the LTCGI results in its own module.

However, thinking about it some more it feels like doing something like using the Toon Ramp and other shading settings might be what you actually want? Then instead of just pure posterization with discrete steps - you could have full ramp-based control over how the diffuse shading from LTCGI is applied? 🤔

In a separate LTCGI module, or using a compiler directive in the current one?
I could see having a separate LTCGI module for Toon could be useful for adding more post effects or configuration to the LTCGI Toon module.

I hadn't really considered a Toon Ramp, though that could be useful and easier to customize.

And yes this effect is primarily for LTCGI on Toon, aiming to make it look more "Toony" to fit with the aesthetic.

@orels1
Copy link
Owner

orels1 commented Mar 5, 2025

In a separate LTCGI module, or using a compiler directive in the current one?

Probably a separate module
So with how v2 toon is set up - every lighting calculation is powered by the module itself, since the Toon shader is essentially just a collection of loosely connected effects.

So for this kinda thing i could imagine LTCGI putting data into something like
half3 customGIDiffuse and half3 customGISpecular, so then you can define your module that runs late which then modifies those in some way. Before the shader lighting model composites them all together

@fundale
Copy link
Author

fundale commented Mar 7, 2025

Gave Toon it's own LTCGI module with Texture Ramp support.
Tried messing around with customGIDiffuse and customGISpecular however they appear to only function in a baked state

@orels1
Copy link
Owner

orels1 commented Mar 7, 2025

oh I meant that I could set up something like that for the 2.0 version, not the current one

@fundale
Copy link
Author

fundale commented Mar 10, 2025

The customGI* or the Ramping / separate module?

@orels1
Copy link
Owner

orels1 commented Mar 11, 2025

the customGI part

@fundale
Copy link
Author

fundale commented Mar 11, 2025

Ah ok

@orels1
Copy link
Owner

orels1 commented Mar 18, 2025

The v7.0.0-dev.8 of the shaders now has support for customGISpecular and customGIDiffuse which you can access via %CustomGI functions, similar to how LTCGI module did it on Dev already.

This is also supported by the Toon shader now.
I would recommend also adding a very high order number to the posterization module since you'd want it to always run last, e.g.

%CustomGI("LTCGICustomGIPosterize", 1000)
{
    void LTCGICustomGI(MeshData d, SurfaceData o, inout half3 customGISpecular, inout half3 customGIDiffuse)
    {
        // modify custom GI stuff before it gets composited with the rest of the lighting
    }
}

@fundale
Copy link
Author

fundale commented Mar 19, 2025

Updated the Toon LTCGI and CustomGI Ramp modules, thought having it as a separate module would become useful for providing Toon Ramping for other CustomGI FX (perhaps AreaLit and VRSLGI).

Also should I remove the LTCGI Toon module from V1?

@orels1
Copy link
Owner

orels1 commented Mar 19, 2025

Have you tested it in Toon v1? As I'm pretty sure toon v1 doesn't have any hook points for %CustomGIFunctions in the fragment function 🤔

@fundale
Copy link
Author

fundale commented Mar 20, 2025

I have not tested the V2 LTCGI Toon Module with V1 Toon,
I'll check and make sure the V1 LTCGI Toon module still works with V1 Toon

@fundale
Copy link
Author

fundale commented Mar 20, 2025

The V1 LTCGI Toon Module still functions with V1 Toon, using %Color however it doesn't work with %CustomGI

@orels1
Copy link
Owner

orels1 commented Mar 20, 2025

yeah, ok, that's about what I expected, as Toon v1 doesnt have %CustomGI hooks

Comment on lines 111 to 118
if (_LTCGIClampBrightness) {
half3 hsv = RGB2HSV(ltcgiData.specular);
hsv.z = tanh(hsv.z) * max(0, _LTCGIMaxBrightness);
indirectSpecular += HSV2RGB(hsv) * _LTCGISpecularIntensity;
} else {
indirectSpecular += ltcgiData.specular * _LTCGISpecularIntensity;
}
indirectDiffuse += ltcgiData.diffuse * _LTCGIDiffuseIntensity;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just getting back to this. Shouldn't this also include all the ramp stuff like you did in the %%Color block?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that is ever used in Toon V1? but I'll include it anyways.

Comment on lines 118 to 162
%Color("LTCGIColor")
{
void LTCGIColor(MeshData d, FragmentData i, inout half4 FinalColor, bool facing)
{
#if defined(ORL_LIGHTING_MODEL_VFX)
{
#if defined(PLAT_QUEST)
if (!_LTCGIEnableOnMobile) return;
#endif

#if defined(INTEGRATE_LTCGI)
LTCGIAccumulatorStruct ltcgiData = (LTCGIAccumulatorStruct) 0;

float3 viewDir = d.worldSpaceViewDir;
if (!facing)
{
viewDir = -d.worldSpaceViewDir;
}

LTCGI_Contribution(
ltcgiData,
d.worldSpacePosition.xyz,
d.worldNormal.xyz,
viewDir,
saturate(0.5 + _LTCGIRoughnessModifier),
d.uv1.xy
);

if (_LTCGIClampBrightness) {
half3 hsv = RGB2HSV(ltcgiData.specular);
hsv.z = tanh(hsv.z) * max(0, _LTCGIMaxBrightness);
FinalColor.rgb += HSV2RGB(hsv) * _LTCGISpecularIntensity;
} else {
FinalColor.rgb += ltcgiData.specular * _LTCGISpecularIntensity;
}
FinalColor.rgb += ltcgiData.diffuse * _LTCGIDiffuseIntensity;
if (_LTCGIAlphaPremultiply)
{
FinalColor.rgb *= FinalColor.a;
}
#endif
}
#endif
}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed? 🤔
wouldn't the main LTCGI module (non-toon) already handle that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main LTCGI module could with a compiler directive,
Toon needs "d.worldNormal.xyz" and has no "o.Smoothness",
Other than that it's pretty much the same.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does Toon V2 provide a directive?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as of a couple dev releases ago, its the same as TOON, but with V2 at the end
ORL_LIGHTING_MODEL_TOON_V2

However, my point here was that this part is limited to ORL_LIGHTING_MODEL_VFX and im not sure why do we need a separate VFX-only block in a toon version of LTCGI, when the main LTCGI already works with VFX

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need a Toon V2 specific variant of LTCGI, that is an oversight on my part.

I was referring to having a slightly different function call in the existing LTCGI (removing the LTCGI Toon V2), that would use d.worldNormal instead of d.normal and skip o.Smoothness as it is currently not defined in the Toon V2 base.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean i dont mind a toon v2 specific module, here I meant specifically that the Toon v2 version of the module for some reason has code that is ifdefed under VFX, which you'd never use the toon LTCGI module with, is all
image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't see that, will look into that then.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's from the original LTCGI module for PBR, I didn't realize it wasn't being used anymore.

Also why does the PBR LTCGI Module use NORMAL instead of WORLDNORMAL?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the "PBR" module is meant to be used for both PBR and VFX, only Toon needed special treatment. So you dont need to add a Color block for just VFX in the Toon v2. As it is only meant to be used in the Toon v2 lighting model.

As for Normal vs WorldNormal

Both of them are World Normals inside the FragmentBase function, the "correct" move there would be to do what I did in toon v2 and have Normal be Tangent Normal and then WorldNormal be the.. well world normal.

But due to lack of foresight - i just converted o.Normal to world space up-top in the PBR Fragment Base, as for the longest time - there was no way to extend the actual FragmentBase function from an outside module. And then i saved the original o.Normal (which is what the individual modules interact with) into tangentNormal at the top of FragmentBase as well

Then when I wrote Toon v2, since the whole lighting function is modularized - i ended up adding another surface struct property - WorldNormal and left Normal untouched, so its very clear which is which.

But that in turn created conflicts with CustomGI modules. Which are unfortunate, but I generally think that it was the better choice (and I might just change the PBR to use the new naming as well? but im still thinking about it)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I've removed the %Color from Toon V2 and %CustomGI from Toon V1,
And model compiler directive checks for both seeing as they should be unused now.

@orels1 orels1 merged commit 7a1d43f into orels1:dev May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants