Skip to content

Conversation

@BlueCube3310
Copy link
Contributor

@BlueCube3310 BlueCube3310 commented Aug 25, 2025

Modifies the code that handles denoising to always pack the L1 coefficients first, then denoise the directional lightmaps. This slightly improves quality and eliminates errors that occur due to dealing with negative color values.

TODO:

  • Verify that this approach doesn't alter the directional data,
  • Test with OIDN

@jcostello
Copy link
Contributor

Seems that directional data is preserved and works just fine with OIDN. It fixed the issue of losing directional data when denoising with OIDN

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected. Code looks good to me.

However, JNLM now has a tendency to darken bright static lights, as seen below. This doesn't happen with OIDN.

Testing project: test_pr_109972.zip

No denoiser JNLM before JNLM after
Screenshot_20250826_180751_no_denoiser Screenshot_20250826_180756_jnlm Screenshot_20250826_181016_jnlm_after
OIDN before OIDN after "Ground truth"1
Screenshot_20250826_180811_oidn Screenshot_20250826_181029_oidn_after Screenshot_20250826_180845_ground_truth

Footnotes

  1. Ultra quality, supersampling 2.0, no denoising

@BlueCube3310
Copy link
Contributor Author

However, JNLM now has a tendency to darken bright static lights, as seen below. This doesn't happen with OIDN.

This can probably be fixed by adjusting the denoiser strength, since the directional data is now packed that value can be a constant. The L0 coefficient has 1/4 the brightness of a non-directional lightmap, so an adjustment may be necessary there as well.

@Repiteo Repiteo requested a review from a team October 24, 2025 15:48
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@clayjohn
Copy link
Member

@BlueCube3310 Should we go ahead and merge this, or did you want to poke at the JNLM issue first?

@BlueCube3310
Copy link
Contributor Author

@BlueCube3310 Should we go ahead and merge this, or did you want to poke at the JNLM issue first?

I'll attempt to fix the issue first

@BlueCube3310
Copy link
Contributor Author

BlueCube3310 commented Oct 31, 2025

The L0 light weights are now multiplied by 4 to ensure the directional lightmap has the same influence as a regular one. Additionally, the L1 coefficients now use the weights from the L0 layer.

I've also fixed a bug that caused shadowmasks to be unnecessarily denoised 4 times when baking with both them and directional enabled.

CC @Calinou could you confirm whether this fixes the JNLM denoising? (I'm no longer able to download the linked MRP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants