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

Catch and report error when IBL map fails to load #8303

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Conversation

OmarShehata
Copy link
Contributor

This came up on the forum. If you take the old_venetian_crossroads_2k_ibl in this zip, and try to load it in the IBL Sandcastle it fails to load, and the only indication you get is that the models are just completely black.

EnvironmentMap.zip

I eventually realized CesiumJS was failing to load that KTX file because it had an unsupported glInternalFormat of R11F_G11F_B10F. This PR gets OctahedralProjectedCubeMap to reject its readyPromise when it fails to load the environment map, and gets Model.js to listen on that and console.error it out. I also added a test to make sure the promise is rejected when it tries to load an invalid URL.

This KTX file was generated from cmgen 1.4.1, but it seems to have this unsupported format because of some recent change. I tested with cmgen 1.2.0 and it works in CesiumJS (included in the above zip).

@cesium-concierge
Copy link

Thanks for the pull request @OmarShehata!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

@cesium-concierge
Copy link

Thanks again for your contribution @OmarShehata!

No one has commented on this pull request in 30 days. Maintainers, can you review, merge or close to keep things tidy?

I'm going to re-bump this in 30 days. If you'd like me to stop, just comment with @cesium-concierge stop. If you want me to start again, just delete the comment.

@emackey emackey merged commit afff7a4 into master Nov 21, 2019
@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/d/msg/cesium-dev/6C8O1faCEFc/CvcK83ZyDgAJ

If this issue affects any of these threads, please post a comment like the following:

The issue at #8303 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.

@emackey emackey deleted the ibl-error branch November 21, 2019 22:28
@OmarShehata
Copy link
Contributor Author

Thanks @emackey !

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