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

GroundProjectedEnv naming is confusing as it's a background, not an environment #25287

Closed
hybridherbst opened this issue Jan 15, 2023 · 2 comments · Fixed by #25645
Closed

GroundProjectedEnv naming is confusing as it's a background, not an environment #25287

hybridherbst opened this issue Jan 15, 2023 · 2 comments · Fixed by #25645

Comments

@hybridherbst
Copy link
Contributor

hybridherbst commented Jan 15, 2023

Description

Since #24311 there is a way to make a ground projected environment - which works great!

But the naming is now somewhat confusing:

  • in three, generally "background" is something visible behind the scene and "environment" is used for e.g. IBL and reflections
  • see the docs: https://threejs.org/docs/#api/en/scenes/Scene.environment vs. Scene.background
  • now, "GroundProjectedEnv" has nothing to do with some PMREM/IBL environment - it should probably be a GroundProjectedBackground and not Env (?)
  • in the sample, it does use the environment and not the background map, which adds to the confusion when someone actually uses a separate background and environment cubemap

Version

latest

@Mugen87
Copy link
Collaborator

Mugen87 commented Jan 15, 2023

We went for GroundProjectedEnv because the technique behind the class is called "ground projected environment maps". According to my personal experience up to this point, GroundProjectedEnv as a module name had a good acceptance so far since especially experienced devs know the technique and thus understand the purpose of the class.

it should probably be a GroundProjectedBackground and not Env (?)

As you have mentioned GroundProjectedEnv is not assigned to Scene.background so I would find this name actually worse. The current one at least describes the technique. But I admit there is a the conflict with Scene.environment...

Maybe GroundProjectedSkybox?

@melMass
Copy link

melMass commented Apr 5, 2023

Glad I finally found this issue, I was only seeing references of GroundProjectedEnv everywhere, my IDE wasn't complaining but vite was (@types/three is outdated)

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

Successfully merging a pull request may close this issue.

3 participants