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

fix(Shader): fix condition statement for gradient-based shadow #2493

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

JiayiXuu
Copy link
Collaborator

Context

Shader doesn't compile when VolumetricScatteringBlending is set to 1.0 due to an error in conditional statement.

Results

Fixed issue

Changes

Changed #ifdef statement for gradient based lighting function declaration in vtkVolumeFS.glsl

PR and Code Checklist

  • semantic-release commit messages
  • Run npm run reformat to have correctly formatted code

Testing

  • This change adds or fixes unit tests
  • Tested environment:
    • vtk.js: latest master
    • OS: Windows 10
    • Browser: Chrome 89.0.4389.128

@JiayiXuu JiayiXuu requested a review from floryst June 28, 2022 21:40
@JiayiXuu
Copy link
Collaborator Author

@floryst could you review the fix for this minor bug? Thank you!

@JiayiXuu JiayiXuu marked this pull request as ready for review June 28, 2022 21:41
@floryst
Copy link
Collaborator

floryst commented Jun 28, 2022

LGTM. Does defined() not work in an if statement?

@JiayiXuu
Copy link
Collaborator Author

LGTM. Does defined() not work in an if statement?

Thanks! The syntax is correct, it's just #ifdef SurfaceShadowOn need to enclose the entire #if vtkLightComplexity < 3 statement, a slight logic difference.

@floryst
Copy link
Collaborator

floryst commented Jun 28, 2022

Ah, I see now. There's an else statement hidden in the collapsed code block. Thanks for the clarification!

@floryst floryst merged commit 6ae17f8 into master Jun 28, 2022
@floryst floryst deleted the fixBlending branch June 28, 2022 22:05
@github-actions
Copy link

🎉 This PR is included in version 25.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Automated label label Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Automated label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants