Optional texture embedding #172
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…dding texture files into the generated vsg native file. This is done by setting the external_textures option to true. Note that this does not work for textures already embedded within the source model! In addition, by suppling the external_texture_format command line option, it is possible to specify whether the stand-alone texture files should be saved in the vsg-native format (either vsgt or vsga). By dong this there is no need for the integrating application to link against vsgXchange for texture conversion.
| root->setObject("external", converter.externalObjects); | ||
|
|
||
| return root; | ||
| } |
There was a problem hiding this comment.
The above code block changes what happens when a null ref_ptr is returned from convert,visit(), and would cause the code to fall through to the vsg::read() at the end of method. Is this intended?
I would have thought a else return {} would keep the original behavior.
There was a problem hiding this comment.
Yes you are correct. I mentally completely blanked out the #if .. #endif block and its contained 'return' statement (without that block the behaviour is actually unchanged, but still probably less easy to read).
I hope to get around to updating this PR beginning of next week.
There was a problem hiding this comment.
I updated the PR now
This was referenced Jul 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The vsgXchange::assimp ReaderWriter now offers the option of not embedding texture files into the generated vsg native file. This is done by setting the external_textures option to true. Note that this does not work for textures already embedded within the source model!
In addition, by suppling the external_texture_format command line option, it is possible to specify whether the stand-alone texture files should be saved in the vsg-native format (either vsgt or vsga). By dong this there is no need for the integrating application to link against vsgXchange for texture conversion.
This leverages the vsg::External object, see this discussion.
One thing I am unsure about is that currently only the vsg::Image is being loaded from external file, but the associated vsg::Sampler is still embedded within the file and not recalculated if the image is being loaded, which could be problematic if that external file has some properties changed (like the number of included mipmaps..)