KHR_materials_openpbr #17200
KHR_materials_openpbr #17200
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/17200/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/17200/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/17200/merge#BCU1XR#0 |
|
Devhost visualization test reporter: |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
| * @internal | ||
| */ | ||
| public _loadMaterialAsync( | ||
| private async _ensurePbrMaterialClassesAsync(): Promise<void> { |
There was a problem hiding this comment.
We have a Lazy helper class now (added recently) that can be used for this type of thing. The Lazy instance can be created at loader construction time and then just accessed in _loadMaterialAsync. Up to you, but I suspect it would be a bit cleaner.
| public _loadMaterialAsync( | ||
| private async _ensurePbrMaterialClassesAsync(): Promise<void> { | ||
| if (!this._pbrMaterialClass || !this._pbrMaterialAdapterClass) { | ||
| const openpbrExt = this._extensions.find((extension) => extension.name === "KHR_materials_openpbr"); |
There was a problem hiding this comment.
Won't this always be true since it is just checking if we have a loader extension for KHR_materials_openpbr (which we always would)? Should this instead be checking the glTF json, e.g.:
| const openpbrExt = this._extensions.find((extension) => extension.name === "KHR_materials_openpbr"); | |
| const openpbrExt = this.isExtensionUsed("KHR_materials_openpbr"); |
There was a problem hiding this comment.
this._extensions appears to be the list of extensions used in the loading glTF, not a complete list of all extensions. So this seems to work fine.
However, your suggested code is certainly cleaner.
|
Devhost visualization test reporter: |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
@bghgary , here's a rough implementation of the extension that would enable OpenPBR per material. Feedback welcome.