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

Grounded Skybox #27448

Merged
merged 4 commits into from
Dec 28, 2023
Merged

Grounded Skybox #27448

merged 4 commits into from
Dec 28, 2023

Conversation

elalish
Copy link
Contributor

@elalish elalish commented Dec 27, 2023

Fixes #27422

Description

As requested; I've simplified it a bit too, as well as removed the GUI from the example since the parameters are not really meant to be tweaked realtime.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 27, 2023

The E2E screenshot needs an update via npm run make-screenshot webgl_materials_envmaps_groundprojected.

@marcofugaro
Copy link
Contributor

Maybe it makes sense to convert the HDR to .jpg like in #27183 for this particular example?

@elalish
Copy link
Contributor Author

elalish commented Dec 27, 2023

Maybe it makes sense to convert the HDR to .jpg like in #27183 for this particular example?

Probably, but let's leave that to another PR. Ideally once UltraHDR support comes to core - 🙏 @mrdoob!

@elalish
Copy link
Contributor Author

elalish commented Dec 27, 2023

Hmm, make-screenshot gave me a black image. It used to work fine on this macbook, but it also used to open Chrome - it seems to be headless now? I wonder if that's related...

Meantime, can someone else generate the screenshot?

@mrdoob mrdoob added this to the r161 milestone Dec 28, 2023
@mrdoob mrdoob merged commit 36f9019 into mrdoob:dev Dec 28, 2023
9 of 11 checks passed
@mrdoob
Copy link
Owner

mrdoob commented Dec 28, 2023

Seems like it worked on my machine... fc885c5 🤔

@hybridherbst
Copy link
Contributor

hybridherbst commented Jan 4, 2024

@elalish is there a reason for not setting depthWrite = false on the material? Can I make a PR?

I adjusted depthWrite in this PR as well:

@elalish elalish deleted the groundedSkybox branch January 8, 2024 20:56
@Methuselah96 Methuselah96 mentioned this pull request Jan 15, 2024
45 tasks
AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
* initial change

* mrdoob approves

* updated example

* update screenshot
@timothyallan
Copy link

Fixes #27422

Description

As requested; I've simplified it a bit too, as well as removed the GUI from the example since the parameters are not really meant to be tweaked realtime.

I run a builder type site where people used to be able to tweak the parameters in real time to get their content to display the way they wanted. Is the best way now to just delete/recreate via new GroundedSkybox() when updating params?

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 17, 2024

Yes. Remove the skybox from the scene and then call dispose() on its geometry and material to make sure to clean things up. You can then setup a new instance.

@timothyallan
Copy link

Yep, the types showed there was no .dispose(), but after checking out the code and seeing how the new code is actually structured, that's exactly what I'm doing now. Thanks.

@mrdoob
Copy link
Owner

mrdoob commented Feb 17, 2024

I run a builder type site where people used to be able to tweak the parameters in real time to get their content to display the way they wanted.

@timothyallan do you have a link to that site?

@timothyallan
Copy link

@timothyallan do you have a link to that site?

It's part of a new suite of 3d tooling being added to an existing SaaS platform. We've been beta testing it all for the past few months, and it's not available to the general public yet. It's good this update happened now actually!

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