Uses decoded URI directly for filename extraction#521
Uses decoded URI directly for filename extraction#521mklefrancois wants to merge 8 commits intosyoyo:releasefrom
Conversation
|
Hi, does this PR looks good ? |
|
please answer Copilot's comment |
|
Could you please add unit test code to |
|
I have added a unit test and found that when saving the .gltf to a different path, the sub-directory wasn't created and therefore failing to write the scene properly. There is a change to the PR where sub-directories will be created if needed. With this change, the unit test passes. |
|
Thank you! Let me give time to review PR |
|
Hi, just checking for the status of the PR. |
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the image filename handling in the TinyGLTF library to preserve subdirectory paths when writing image files. Instead of extracting only the base filename from decoded URIs, the code now uses the complete decoded URI as the filename, allowing subdirectories to be preserved.
- Replaces
GetBaseFilename(decoded_uri)with direct use ofdecoded_urifor filename extraction - Adds a new
CreateDirectoriesfunction to recursively create directory structures - Integrates directory creation into the file writing workflow to support subdirectory preservation
Reviewed Changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tiny_gltf.h | Core implementation changes including new directory creation function and modified filename handling |
| tests/tester.cc | Test case to verify subdirectory path preservation functionality |
| models/CubeWithSubDir/README.md | Documentation for test model with subdirectory structure |
| models/CubeWithSubDir/Cube.gltf | Test model file containing image URIs with subdirectory paths |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
there is also unresolved unresolved review. |
The GetBaseFilename function was previously used to extract the base filename from the decoded URI. However, the decoded URI already represents the desired filename. This allow to preserve the sub-directory of the original URI.
Ensures that image URI paths, including subdirectories, are correctly preserved when writing glTF files. This prevents issues where texture paths are flattened, leading to incorrect file access. Adds recursive directory creation to ensure that subdirectories specified in image URIs are created during the write process. Includes a new test case to verify the correct preservation of image URI paths with subdirectories.
mklefrancois
force-pushed
the
image_uri_fix
branch
from
24cd853 to
b35a81e
Compare
|
/copilot review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 17 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
The
GetBaseFilenamefunction was previously used to extract the base filename from the decoded URI. However, the decoded URI already represents the desired filename. This allow to preserve the sub-directory of the original URI.See issue: https://github.com/syoyo/tinygltf/issues/520