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

Unify envmap textures and deprecate sphericalEnvMap #5454

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Feb 9, 2024

Description:
Follow-up of #5453, also preparation work for #5449. In A-Frame environment maps were handled separately in both the standard and phong shader. Besides the code-duplication this also meant that there was no texture re-use between them or other textures.

This PR moves the envMap logic into utils/material and systems/material in a similar vein to other textures. The eventual texture loads now also go through the texture cache. The possible values for envMap has been broadened to support both cubemaps and equirectangular maps. The latter allows a fallback for the currently incorrect behaviour of sphericalEnvMap. There is one small breaking change, the envMap value now behaves closer to asset property types, meaning it now only supports ID based selectors and would otherwise assume a URL in case it's unwrapped (e.g. breaking envMap="#someElement > a-cubemap.active").

Changes proposed:

  • Move loading of envMap to a common implementation in utils/material and systems/material making use of existing texture loading and caching.
  • Expand the possible values for envMap to support both cubemap (six images/<a-cubemap>) as well as Equirectangular images.
  • Deprecate sphericalEnvMap as Three.js dropped support for them long ago and A-Frame incorrectly uses them as Equirectangular images. Fallback is in place, logging a warning, but otherwise ensuring those expecting the equirectangular behaviour won't notice a regression.
  • Added unit tests for utils/src-loader and shaders/phong

@mrxz mrxz force-pushed the unify-envmap-textures branch from 007b6df to 90ead38 Compare February 9, 2024 16:41
@dmarcos
Copy link
Member

dmarcos commented Feb 9, 2024

Thanks!

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