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

Examples: Modularized PMREM and added webgl2_loader_gltf #16072

Merged
merged 3 commits into from
Mar 27, 2019

Conversation

oparisy
Copy link
Contributor

@oparisy oparisy commented Mar 27, 2019

Following discussions with @donmccurdy, this PR provides a fully functional version of the glTF loader example using modules from examples/jsm.

For this purpose I needed to include the examples/js/PMREM utils to the modularize.js script provided by #9562. This PR also provides .d.ts type definitions for those utilities.

Following @mrdoob advice, I named this example webgl2_loader_gltf.html since webgl2 samples already use ES6 syntax. I chose to use a coding style closer to the examples/jsm files though (only import needed modules from three.module.js, no THREE prefix).

I also took special care to minimize differences with the original webgl_loader_gltf.html, so that they can be diffed for reference purpose.

I'd especially appreciate a code review on this coding style, and the provided type definitions.

@mrdoob mrdoob added this to the r103 milestone Mar 27, 2019
@mrdoob mrdoob merged commit 9f7f38b into mrdoob:dev Mar 27, 2019
@mrdoob mrdoob changed the title Modularize webgl loader gltf Examples: Modularized PMREM and added webgl2_loader_gltf Mar 27, 2019
@donmccurdy
Copy link
Collaborator

This should be using WebGL2Renderer rather than WebGLRenderer – I tried making that change but:

  1. WebGL2Renderer is not currently exported by the ES module version (easy to fix)
  2. WebGL2Renderer is not compatible with PMREMGenerator (??? to fix)

@mrdoob
Copy link
Owner

mrdoob commented Mar 27, 2019

I'm hesitant about WebGL2Renderer. At this point, seems like it'll be easier to just make WebGLRenderer use WebGL 2.0 when available. If we're going to create a new renderer from scratch, maybe better to focus on WebGPURenderer instead.

@donmccurdy
Copy link
Collaborator

Ah ok. In the meantime, should we hide this example from the sidebar until it uses webgl2, or list it elsewhere?

@mrdoob
Copy link
Owner

mrdoob commented Mar 27, 2019

Yeah, we should do some clean up next cycle.

@oparisy
Copy link
Contributor Author

oparisy commented Mar 27, 2019

May I suggest an ES6 or JSM prefix instead of webgl2 then?

@oparisy oparisy deleted the modularize-webgl-loader-gltf branch March 31, 2019 09:19
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.

3 participants