[Merged by Bors] - gltf: load normal and occlusion as linear textures #1762
Conversation
This should hopefully also resolve the issues outlined in the Mipmaps PR: #1685 , as they were also caused by this asymmetry / different load path for textures in GLTF/GLB leading to sampler parameters being ignored. |
| gltf::image::Source::Uri { uri, .. } => { | ||
| let parent = load_context.path().parent().unwrap(); | ||
| let image_path = parent.join(uri); | ||
| let asset_path = AssetPath::new(image_path, None); |
There was a problem hiding this comment.
With this PR no longer using the asset system to load external texture files (and not tracking the image path / not using it for the handle), and not adding them as asset dependencies, I suspect that might mean that the asset system is no longer going to detect if any of the referenced texture files is modified, which means that they will not be hot-reloaded.
(Instead, modifying the GLTF itself would now hot-reload everything, with all the external textures, as they are treated the same as embedded GLB textures.)
Further, we lose the performance/parallelism benefits that we had before, when textures were queued up to be loaded asynchronously by the asset system.
There was a problem hiding this comment.
Without a large rework, the asset server is not able to load this kind of assets with external configuration. I think losing the hot-reload of parts of a gltf in exchange for it being correctly loaded is a gain.
Longer term, the Distill integration should provide this feature.
For the async part, I have something working locally (that still need some light work), but would prefer this and #1632 merged first to avoid having too many pr touching the same parts at the same time.
There was a problem hiding this comment.
Yeah, the current asset system is kinda limited and quirky, really looking forward to Distill. For now we have to make do with what we have, and I agree that loading and rendering the assets correctly should take priority over the features of the asset system working smoothly.
I am excited about your improvement/fix for async loading, sounds like it should address the performance issues when loading big GLBs with many large textures.
There was a problem hiding this comment.
Yeah I think this is the right short term move. I don't think async will solve the performance issues with image loading. We aren't queuing up multiple async tasks at the same time, so while it might run the async task on a separate thread, we won't start the next one until the previous one is finished.
There was a problem hiding this comment.
Ahh nobody was claiming the current async stuff was a perf improvement. My bad!
Ratysz
added
the
A-Assets
|
bors r+ |
Load textures from gltf as linear when needed. This is for #1632, but can be done independently and won't have any visible impact before. * during iteration over materials, register textures that need to be loaded as linear * during iteration over textures * directly load bytes from external files instead of adding them as dependencies in the load context * configure the texture the same way for buffered and external textures * if the texture is linear rgb, set as linear rgb
|
Pull request successfully merged into main. Build succeeded: |
bors
bot
changed the title
Adds an GitHub Action to check all local (non http://, https:// ) links in all Markdown files of the repository for liveness. Fails if a file is not found. # Goal This should help maintaining the quality of the documentation. # Impact Takes ~24 seconds currently and found 3 dead links (pull requests already created). # Dependent PRs * #2064 * #2065 * #2066 # Info See [markdown-link-check](https://github.com/marketplace/actions/markdown-link-check). # Example output ``` FILE: ./docs/profiling.md 1 links checked. FILE: ./docs/plugins_guidelines.md 37 links checked. FILE: ./docs/linters.md [✖] ../.github/linters/markdown-lint.yml → Status: 400 [Error: ENOENT: no such file or directory, access '/github/workspace/.github/linters/markdown-lint.yml'] { errno: -2, code: 'ENOENT', syscall: 'access', path: '/github/workspace/.github/linters/markdown-lint.yml' } ``` # Improvements * Can also be used to check external links, but fails because of: * Too many requests (429) responses: ``` FILE: ./CHANGELOG.md [✖] #1762 → Status: 429 ``` * crates.io links respond 404 ``` FILE: ./README.md [✖] https://crates.io/crates/bevy → Status: 404 ```
Adds an GitHub Action to check all local (non http://, https:// ) links in all Markdown files of the repository for liveness. Fails if a file is not found. # Goal This should help maintaining the quality of the documentation. # Impact Takes ~24 seconds currently and found 3 dead links (pull requests already created). # Dependent PRs * bevyengine#2064 * bevyengine#2065 * bevyengine#2066 # Info See [markdown-link-check](https://github.com/marketplace/actions/markdown-link-check). # Example output ``` FILE: ./docs/profiling.md 1 links checked. FILE: ./docs/plugins_guidelines.md 37 links checked. FILE: ./docs/linters.md [✖] ../.github/linters/markdown-lint.yml → Status: 400 [Error: ENOENT: no such file or directory, access '/github/workspace/.github/linters/markdown-lint.yml'] { errno: -2, code: 'ENOENT', syscall: 'access', path: '/github/workspace/.github/linters/markdown-lint.yml' } ``` # Improvements * Can also be used to check external links, but fails because of: * Too many requests (429) responses: ``` FILE: ./CHANGELOG.md [✖] bevyengine#1762 → Status: 429 ``` * crates.io links respond 404 ``` FILE: ./README.md [✖] https://crates.io/crates/bevy → Status: 404 ```
Load textures from gltf as linear when needed.
This is for #1632, but can be done independently and won't have any visible impact before.