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 aquarium demo #29

Merged
merged 1 commit into from
Aug 4, 2024
Merged

Fix aquarium demo #29

merged 1 commit into from
Aug 4, 2024

Conversation

rndexe
Copy link
Contributor

@rndexe rndexe commented Jul 30, 2024

  1. Fix AccumulativeShadows and RandomizedLight intensity
  2. Fix useMask not working as mentioned in this issue
    useMask hook and Mask component doesn't work with the current version of three.js drei#1949
  • ready for merge?

@@ -8,7 +8,7 @@ import turtleModel from './model_52a_-_kemps_ridley_sea_turtle_no_id-transformed

export default function App({ spheres }) {
return (
<Canvas shadows camera={{ position: [30, 0, -3], fov: 35, near: 1, far: 50 }}>
<Canvas shadows camera={{ position: [30, 0, -3], fov: 35, near: 1, far: 50 }} gl={{ stencil:true }}>
Copy link
Member

Choose a reason for hiding this comment

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

is this gl={{ stencil:true }} really necessary in this case? I tried without, seems to have no effect :/

Copy link
Member

Choose a reason for hiding this comment

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

what's default value?

Copy link
Contributor Author

@rndexe rndexe Jul 31, 2024

Choose a reason for hiding this comment

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

Without it the turtle and the spheres were not clipping to the inside of the aquarium. I was using the fix for the issue mentioned in the description.

The default value is false from r163 onwards.
mrdoob/three.js#27900

Copy link
Contributor Author

@rndexe rndexe Jul 31, 2024

Choose a reason for hiding this comment

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

without gl={{ stencil:true }}

Screenshot from 2024-07-31 12-28-02

with gl={{ stencil:true }}

Screenshot from 2024-07-31 12-26-49

@abernier
Copy link
Member

abernier commented Aug 1, 2024

this one should have the CI failed (because this demo has a visual test defined + your edits change the rendering) -- but it didn't and make me realise that the CI still needs tweaks: #34

so I keep it open until I've fixed this ;)

@abernier abernier merged commit c200b6c into pmndrs:main Aug 4, 2024
2 checks passed
@rndexe rndexe deleted the aquarium branch August 4, 2024 11:03
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.

2 participants