-
-
Notifications
You must be signed in to change notification settings - Fork 311
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
Clamp linear gamma at last output #2243
Conversation
WalkthroughThe recent changes introduce modifications to the GLSL shader files. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/core/src/shaderlib/common.glsl (1 hunks)
- packages/core/src/shaderlib/pbr/ibl_frag_define.glsl (1 hunks)
Additional comments not posted (8)
packages/core/src/shaderlib/pbr/ibl_frag_define.glsl (2)
Line range hint
1-13
:
LGTM!The
getLightProbeIrradiance
function is well-implemented and has no changes in this PR.
35-37
: Verify the impact of the conditional check removal.The conditional check removal simplifies the return statement but may affect the function's behavior under specific input conditions.
packages/core/src/shaderlib/common.glsl (6)
Line range hint
8-9
:
LGTM!The
pow2
function is well-implemented and has no changes in this PR.
Line range hint
11-13
:
LGTM!The
RGBMToLinear
function is well-implemented and has no changes in this PR.
Line range hint
15-17
:
LGTM!The
gammaToLinear
function is well-implemented and has no changes in this PR.
19-21
: LGTM!The addition of the clamping line in the
linearToGamma
function enhances stability by ensuring that thelinearIn
vector does not have negative values.
Line range hint
23-25
:
LGTM!The
remapDepthBufferLinear01
function is well-implemented and has no changes in this PR.
Line range hint
28-83
:
LGTM!The matrix inversion functions are well-implemented and have no changes in this PR.
Please check if the PR fulfills these requirements
delete it when support sRGB
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
Other information:
Summary by CodeRabbit
linearToGamma
function to ensure input values are clamped, enhancing stability.envBRDFApprox
function for better performance and clarity.