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

Import KTX2Loader from threejs PR. #172

Merged
merged 4 commits into from Feb 10, 2020
Merged

Import KTX2Loader from threejs PR. #172

merged 4 commits into from Feb 10, 2020

Conversation

donmccurdy
Copy link
Contributor

Imports KTX2Loader from open PR (mrdoob/three.js#18490) to consolidate development in one place. The loader now depends on changes to the threejs library (addition of ETC2 support) which means development is easier to manage over there.

  • Adds an import map shim, to ensure KTX2Loader uses the right threejs CDN during development. The release on CDN does not yet have the ETC2 support.
  • Removes requestIdleCallback invocation, so demo can be tested on Safari.

The eventual goal here is that — once the KTX2Loader is released with threejs — this demo will can demonstrate KTX2 loading with the official threejs release library. If/when we later add a separate pure-JS KTX2 loader, that is engine-agnostic, we can decide how to best publish that.

- Imports KTX2Loader from open PR (mrdoob/three.js#18490) to consolidate development.
- Adds an import map shim, to properly link KTX2Loader to the threejs CDN.
- Removes requestIdleCallback invocation, so demo can be tested on Safari.
@@ -77,22 +81,18 @@ <h2>msc_basis_transcoder test</h2>
[THREE.RGB_ETC1_Format]: "RGB_ETC1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add THREE.RGB_ETC2_Format?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done – although the latest threejs release doesn't have ETC2 support yet, so (regardless of this change) the demo will likely fail on Android for now. I expect the ETC2 support will be released at the end of February.

@MarkCallow
Copy link
Collaborator

From where will users of KTX2Loader.js download msc_basis_transcoder.js? (The index.html here is correctly referring to the local copy.)

@donmccurdy
Copy link
Contributor Author

For now using a local copy of msc_basis_transcoder.js seems best, especially in case any changes are made within this repository. ThreeJS will likely publish a copy in our releases as well, as we do for .basis.

@MarkCallow MarkCallow merged commit 84f972a into KhronosGroup:ktx2 Feb 10, 2020
@MarkCallow
Copy link
Collaborator

MarkCallow commented Feb 10, 2020

Damn. I should have made a branch and tested this before merging. I am getting the message

Loading failed for the <script> with source “https://unpkg.com/es-module-shims@0.4.6/dist/es-module-shims.js”.

in the Firefox web console. That is the only message. There is no indication of why it failed. There is nothing in the browser console either.

I may have to back out this PR.

@donmccurdy
Copy link
Contributor Author

Hm let me check on that, I had tested in Firefox, safari, and chrome earlier.

@MarkCallow
Copy link
Collaborator

In case it matters, I'm running the http-server included with node.js. I don't think it does because there is no sign of the browser attempting to fetch this script from my test server.

@MarkCallow
Copy link
Collaborator

Sorry. My fault. I use NoScript and had not trusted unpkg. Now I've done that, it works.

@donmccurdy donmccurdy deleted the external-three-dep branch February 10, 2020 05:11
MarkCallow pushed a commit to MarkCallow/KTX-Software that referenced this pull request Feb 29, 2020
* Import KTX2Loader from threejs PR.
- Imports KTX2Loader from open PR (mrdoob/three.js#18490) to consolidate development.
- Adds an import map shim, to properly link KTX2Loader to the threejs CDN.
- Removes requestIdleCallback invocation, so demo can be tested on Safari.
* Remove KTX2Loader.
* Add comment to explain import map shim
* Include ETC2 in format labels.
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.

None yet

2 participants