Skip to content

Filmgrain: fix native filmgrain by making it work in approximate PQ space and apply the same fixes to the improved one#21

Open
EndlesslyFlowering wants to merge 287 commits into
masterfrom
big_grain_fix
Open

Filmgrain: fix native filmgrain by making it work in approximate PQ space and apply the same fixes to the improved one#21
EndlesslyFlowering wants to merge 287 commits into
masterfrom
big_grain_fix

Conversation

@EndlesslyFlowering

@EndlesslyFlowering EndlesslyFlowering commented Oct 18, 2023

Copy link
Copy Markdown
Owner

I rewrote both the native filmgrain and @clshortfuse's filmgrain to work in "perceptual space".
I use a approximation of PQ for HDR and leave it in gamma space in SDR.
Filmgrain produces better results in "perceptual space" rather than in linear.

Just realised that for SDR it's not a given that the input is in gamma.
@Filoppi are we going to do SDR in linear until output? or is that still up in the air?

EndlesslyFlowering and others added 30 commits September 30, 2023 01:19
(with some minor changes)
-Inverse tonemap by luminance (to maintain SDR hue)
-Inverse tonemap highlights only (to maintain SDR crushed shadow filmic look)
-Automatically find paper white multiplier by checking how much mid gray shifted with the inverse tonemapper
-Improve LUTs shadow invalid colors
…st of the image will have the same look as SDR

-Improve LUTs shadow gradient
-Remove AutoHDR (it was just a fun test)
-Clean up code
…the LUT already was (HDR only) (they are cached on the side when computing the SDR image)

-Fixed Inverse Hable being unable to restore highlights, by simply using the original color instead, given that there's no post process anymore happening between tm and inv tm
-Improved "INVERT_TONEMAP_HIGHLIGHTS_ONLY" (it was not fully implemented with ACES, and was lacking mid gray adjustments)
-Polish code a bit and remove unnecesary old stuff
Make DICE tonemap per channel (WIP)
…orRatio" having highly unstable values (likely due to tiny values dividing tiny values, producing unstable results).
… which only enables gamma 2.2 emulation after LUT application as the other mode is incorrect
…ensity towards the lower and top end

remove old actual sigmoidal contrast adjustment as it is no longer needed
EndlesslyFlowering and others added 17 commits October 17, 2023 06:04
- add ability to clamp to AP0 for scRGB (currently disabled for testing; should be enabled for release)
- split shader work for FP16 output and UNORM10 output
…from midtones (shadow/toe end) (disabled as it looks bad), and one to not start the DICE highlights tonemapper until hitting 33% of the screen peak brightness (enabled as it looks good under good HDR calibration settings)
… (we still clamp them away for scRGB on output)
…oken, it needs to have gamma applied and it's too complicated to do that in the HDR composite shader ("AdditionalNeutralLUTPercentage") can be used for that
…pace and apply the same fixes to the improved one
const float filmGrainInvSize = 1.f / (FILM_GRAIN_TEXTURE_SIZE - 1u);
const float filmGrainHalfSize = 521.f; //TODO: rename in case we keep this as 521
static const float filmGrainInvSize = 1.f / (FILM_GRAIN_TEXTURE_SIZE - 1);
static const float filmGrainHalfSize = 511.f;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

511 seems to be the correct value as: (1024 / 2) - 1 = 511
this value also produces the least artifacts

@clshortfuse clshortfuse left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In the improved version, as the code is, just removes nuance of grain completely:

Starfield 10_18_2023 1_06_48 AM

Performing the requested changes achieves this look:
Starfield 10_18_2023 1_22_07 AM

Without the PR, the master branch looks like this:

Starfield 10_18_2023 1_07_07 AM


#if 0 // Output Visualization
float oldY = Luminance(GAMMA_TO_LINEAR(gammaColor));
float oldY = Luminance(GAMMA_TO_LINEAR(linearColor));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

linearColor doesn't exist. This should probably be color now.


float3 linearColor = inputColor;
if (isInOutColorLinear) {
linearColor /= PQMaxWhitePoint;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This line seems better removed since without it, the density no longer scales properly.

outputColor = max(outputColor, 0.f);
outputColor = AP0D65_To_BT709(outputColor);

outputColor *= PQMaxWhitePoint;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If removed aboved, the should be removed here as well.

;
* 3.333f; // Bump to 10%

yChange *= 0.0666f;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Needs a comment explaining it's usage (and can be folded into the above declaration. Though, after adjusting for 80 nits, it's better removed.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

was just testing values that looked right ... 😓


float yAdjustment = isHDR
? (HdrDllPluginConstants.HDRGamePaperWhiteNits / WhiteNits_sRGB)
? (HdrDllPluginConstants.HDRGamePaperWhiteNits / PQMaxWhitePoint)

@clshortfuse clshortfuse Oct 18, 2023

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Film density works in a range that is 0 to 3, whilch should be 0-1 in Y space. Middle gray matches RGB Luminance at ~18%. Anything above diffuse white is more or less the same as having no grain. That's why we want SDR to stay as it is, but HDR back down.

See:

image

@EndlesslyFlowering EndlesslyFlowering Oct 19, 2023

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

the logarithm of light intensity is in foot candle in the source
so a density of 3 is achieved at ~3.6. so 10^3.6 = 3981 lm/ft² which is 42851 cd/m²!

not sure why we cap at mid gray then?

I get that it is supposed to cap
but film never has no grain?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

density is based on exposure time and the light intensity

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We're not capping at midgray, we're basing around 0.75 optical density (mid gray). FIlm Grain is a perceived thing, not a physical thing. It depends on different attributes. If there's no density (white on negative), there's no graininess. But at the same time, anything near black output (white on negative), has stronger perceived graininess. The formula for computed perceptual graininess is part of computeFilmGraininess. Once it hits around diffuse white, there's no real perceived grain (near ~0), so we can set bounds around paperwhite.

Right now there's no curve to match exposure to density (eg: film stock). It's just linear, but close enough for the intended effect.

The source code already links to the references.

@Filoppi

Filoppi commented Oct 18, 2023

Copy link
Copy Markdown
Collaborator

Linear (SDR) until output is a branch. For now it's on but let's support both cases.

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.

5 participants