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

fix(composer): hdr url is sometimes wrong #352

Merged
merged 1 commit into from
Jan 9, 2023
Merged

fix(composer): hdr url is sometimes wrong #352

merged 1 commit into from
Jan 9, 2023

Conversation

sheilaXu
Copy link
Contributor

@sheilaXu sheilaXu commented Nov 11, 2022

Overview

The uriModifier for GLTFLoader is applied to load hdr files unexpectedly and modifies the hdr file urls (if not an absolute url) to be from S3. The hdr files are not served from S3 so loading it will fail.

This issue is not always happening because the Environment component is rendered before GLTFComponent, so the set uriModifier code is often executed after hdr is loading already.

Steps to repro:

  1. start the storybook with a scene from AWS account
  2. after the scene is rendered, go to scene settings and choose a different environment preset
  3. could not load hdr file error will be shown

The fix is to create a separate loading manager for GLTTFLoader so that the uriModifier is not updated for all loaders.

Tested with GLTF model that has png as texture. the url for the png is correctly modified to be from S3.

Before fix:

before-url-fix.mov

After fix:

after-url-fix.mov

Legal

This project is available under the Apache 2.0 License.

@sheilaXu sheilaXu force-pushed the fix-hdr-url branch 2 times, most recently from 10fad6e to 1c02722 Compare November 17, 2022 20:11
@sheilaXu sheilaXu force-pushed the fix-hdr-url branch 2 times, most recently from 87c1553 to a586673 Compare December 1, 2022 19:09
@TheEvilDev TheEvilDev marked this pull request as ready for review December 7, 2022 18:07
TheEvilDev
TheEvilDev previously approved these changes Dec 7, 2022
@sheilaXu sheilaXu merged commit 2c2625e into main Jan 9, 2023
@sheilaXu sheilaXu deleted the fix-hdr-url branch January 9, 2023 19:24
@github-actions github-actions bot mentioned this pull request Jan 9, 2023
mukeshsahay pushed a commit that referenced this pull request Jan 11, 2023
author dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 1672181186 -0800
committer Mukesh Sahay <muksahay@amazon.com> 1673468961 -0800

build(deps): bump decode-uri-component from 0.2.0 to 0.2.2 (#419)

Bumps [decode-uri-component](https://github.com/SamVerschueren/decode-uri-component) from 0.2.0 to 0.2.2.
- [Release notes](https://github.com/SamVerschueren/decode-uri-component/releases)
- [Commits](SamVerschueren/decode-uri-component@v0.2.0...v0.2.2)

---
updated-dependencies:
- dependency-name: decode-uri-component
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

chore: upgrade video.js to remove xmldom (#447)

chore(AddSVGLinting): Enforce width and height attributes on SVGs

fix(composer): support Windows dependency file paths in GLTF loader (#417)

* Add example 3D model with textures referenced with Windows paths for testing

fix(composer): Auto expand hierarchy when selecting node on scene

fix(composer): Update scene hierarchy test cases

Revert "fix(composer): Update scene hierarchy test cases"

This reverts commit fcf1666.

Revert "fix(composer): Auto expand hierarchy when selecting node on scene"

This reverts commit 0395c86.

fix(composer): Auto expand hierarchy when selecting node on scene

fix(composer): Add test for SceneHierarchyDataProvider

fix(composer): hdr url is sometimes wrong (#352)

chore: release 2.6.1 (#440)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

fix: prevent stream render if dataType unknown (#448)

chore: release 2.6.2 (#455)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

fix: prevent stream render if dataType unknown (#448)

chore: release 2.6.2 (#455)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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

3 participants