Skip to content

Conversation

@jure
Copy link

@jure jure commented Dec 11, 2020

Description:

The THREE.SphericalReflectionMapping mapping that sphericalEnvMap relies on has been removed in Three.js (mrdoob/three.js#19517) and the library was released without it in r118, so reflections using this feature no longer work in the latest release of A-Frame.

The cited reasons are (and I'm somewhat paraphrasing) that it almost never does what the user expects it to do or is otherwise ill-suited. And that using cube maps or something like https://github.com/colinfizgig/aframe_Components/blob/master/components/camera-cube-env.js, i.e. camera-generated envMaps is a better approach for reflections.

There are three options as I see it:

  1. Removing sphericalEnvMap
  2. Reintroducing THREE.SphericalReflectionMapping in super-three
  3. Some sort of translation layer into cube maps

The PR implements option 1, as the upstream reasons for removal make sense to me, but happy to discuss further!

@jure
Copy link
Author

jure commented Dec 12, 2020

One test is failing/timing out on Firefox: adding AR state. The same test is failing on master, however, so it’s unrelated to this PR.

@jure
Copy link
Author

jure commented Dec 14, 2020

So I did a bit of exploration and it looks like this is used quite a lot in the community: https://github.com/search?q=sphericalEnvMap&type=code. In this case, perhaps the best way would be to provide a translation layer so these keep working? I don't think I described it clearly enough initially: sphericalEnvMap is currently broken in 1.1.0.

@jure jure force-pushed the remove_spherical_env_map branch from ac48c2b to 99acd60 Compare December 21, 2020 21:33
@jure
Copy link
Author

jure commented Dec 21, 2020

Just rebased this upon the latest master, so that specs pass. Still interested in feedback about the actual contents of the PR!

  1. Should we remove sphericalEnvMap as this PR does?
  2. Should we provide a fallback for it, even though Three.js does not support it anymore?
@dmarcos
Copy link
Member

dmarcos commented Jan 5, 2021

Thanks and sorry for the delay. If sphericalEnvMap is gone. Should we have an alternative or at least recommend one in the docs / faq?

@jure
Copy link
Author

jure commented Jan 5, 2021

Three.js's own migration notes include this notice: "SphericalReflectionMapping is no longer supported. Consider using a Matcap texture with MeshMatcapMaterial instead."

For future reference, this is the PR that removed it + reasons: mrdoob/three.js#19517

I think the MeshMatcapMaterial migration suggestion above is for when spherical reflection mapping is "correctly" used - I expect a fair number of projects use it incorrectly though and would be better served by envMaps, so I've updated the docs to suggest envMap as the recommended alternative, as that's probably what people want, anyway:

Notice: The previously available sphericalEnvMap property was removed because support no longer exists in Three.js (PR): we recommend users switch to envMap for environment reflections.

I'm not 100% sure though - let me know if you think a different approach would be better, happy to fix it!

@dmarcos
Copy link
Member

dmarcos commented Dec 7, 2021

@jure I know this is a bit old. Maybe we're in a better position now to make a decision. Do you think we should finally remove spherical maps or offer an alternative? Are cube maps the way to go and a suitable replacement? Should we switch to MeshMatcapMaterial as recommended in the migration guide?

@dmarcos
Copy link
Member

dmarcos commented Nov 12, 2024

closing due to inactivity

@dmarcos dmarcos closed this Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants