Skip to content

Conversation

@richardmonette
Copy link
Contributor

@richardmonette richardmonette commented May 24, 2018

img_0186 copy

  • Fixes a weird thing where the EquiangularToCubeGenerator had the format hard coded (it's now read from the source by default).

  • Adds overrides for the type and format, specifically this makes it possible to tweak the webgl_materials_envmaps_exr example to work on mobile webGL (where only half precision texture is supported) Before the textures showed up as blank/black, see screenshot above for it now working nicely.

@WestLangley

@WestLangley
Copy link
Collaborator

Agreed.

I'm thinking that a more flexible API would be

EquiangularToCubeGenerator( sourceTexture, resolution, options );

where options is an object.

If you don't think that flexibility is necessary, then -- not to derail this thread -- I think we need to revisit the THREE.EXRLoader() output texture options. What should the output mips, encoding, and min/mag filters be for THREE.EXRLoader()?

@richardmonette
Copy link
Contributor Author

What should the output mips, encoding, and min/mag filters be for THREE.EXRLoader()?

Good question. Perhaps we can take that into another discussion, and still 🚢this spot fix. I've updated to the options object, as suggested 👍

@WestLangley
Copy link
Collaborator

@richardmonette Thanks! The API change should be noted in the migration docs, especially if there is no console warning.

@mrdoob This looks good.

@richardmonette
Copy link
Contributor Author

The API change should be noted in the migration docs

Happy to update this, can you please point me towards those docs? (Haven't ever had to update those yet! 😄)

@WestLangley
Copy link
Collaborator

@telime
Copy link

telime commented Jul 18, 2018

oh disagreement... I am looking forward to it.
"Renamed EquiangularToCubeGenerator to EquirectangularToCubeGenerator"

@mrdoob
Copy link
Owner

mrdoob commented Jul 31, 2018

Ops! Any chance you can resolve the conflicts?

@mrdoob mrdoob added this to the r96 milestone Jul 31, 2018
@mrdoob mrdoob merged commit 94743d1 into mrdoob:dev Aug 30, 2018
@mrdoob
Copy link
Owner

mrdoob commented Aug 30, 2018

Thanks!

@WestLangley
Copy link
Collaborator

The API change should be noted in the migration docs, especially if there is no console warning.

Done.

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.

4 participants