Skip to content
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

Potential divide by zero in Bloom shader #15

Open
oncer opened this issue Oct 31, 2019 · 2 comments
Open

Potential divide by zero in Bloom shader #15

oncer opened this issue Oct 31, 2019 · 2 comments
Assignees
Labels

Comments

@oncer
Copy link

oncer commented Oct 31, 2019

If you look inside Assets/Kino/Bloom/Shader/Bloom.cginc in the prefilter part:

    // Under-threshold part: quadratic curve
    half rq = clamp(br - _Curve.x, 0, _Curve.y);
    rq = _Curve.z * rq * rq;

    // Combine and apply the brightness response curve.
    m *= max(rq, br - _Threshold) / max(br, 1e-5);

Half is a 16-bit floating number on some platforms (see Unity docs). The smallest "normal" number for a 16-bit float is 0.000061035, so 1e-5, or 0.00001 is either denormalized or flushed to zero, depending on the shader compiler, potentially resulting in nan values for fragments with calculated brightness of 0.
I can confirm this happens on Nintendo Switch, probably some mobile platforms too. The areas with nan values in the prefilter texture will get passed down in the downsample/upsample passes and result in black rectangles in the final renderbuffer.
Changing this value from 1e-5 to 6.2e-5 resolves the issue.

@oncer oncer changed the title Potential underflow (denormalized number flush to zero) in Bloom shader Potential divide by zero in Bloom shader Oct 31, 2019
@keijiro keijiro self-assigned this Oct 31, 2019
@keijiro
Copy link
Owner

keijiro commented Oct 31, 2019

Thanks for pointing it out. I think this has been fixed in the post-processing stack. I'm not using this effect (KinoBloom) any more, but I'll fix it if there is any chance to update this for some reason.

@larssteenhoff
Copy link

Thanks for the fix, I'm still using it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants