GLTFUnarchiver + GLTFSceneSource: add embedExternalImages option #34
Conversation
| @@ -35,14 +35,16 @@ public class GLTFUnarchiver { | |||
| private var buffers: [Data?] = [] | |||
| private var materials: [SCNMaterial?] = [] | |||
| private var textures: [SCNMaterialProperty?] = [] | |||
| private var images: [Image?] = [] | |||
| // An array of Image or URL types | |||
| private var images: [Any?] = [] | |||
There was a problem hiding this comment.
Please note that I’m new to Swift, and my most recent experience with typed languages is TypeScript. Unfortunately I couldn’t declare this as an array of Image | URL types. If Any is unacceptable, I would consider this instead:
| private var images: [Any?] = [] | |
| private var images: [Image?] = [] | |
| private var imageUrls: [URL?] = [] |
| @@ -771,7 +775,8 @@ public class GLTFUnarchiver { | |||
| return valueArray | |||
| } | |||
|
|
|||
| private func loadImage(index: Int) throws -> Image { | |||
| // Returns either an Image or a URL | |||
| private func loadImage(index: Int) throws -> Any { | |||
There was a problem hiding this comment.
Similar to the above comment regarding Any, I am willing to revise this part of the code to avoid Any as the return type. I would consider factoring out the shared logic here in loadImage() and then create a second private func loadImageUrl(index: Int) throws -> URL method.
| image = try loadImageFile(from: url) | ||
| // Even if we shouldn't embed external images, try loading the image file for now | ||
| // TODO: figure out how to check if `url` exists on-disk rather than using `loadImageFile()` to do this | ||
| let loadedImage = try loadImageFile(from: url) |
There was a problem hiding this comment.
This is the most awkward part of this revision. As indicated in the comments, I’m essentially using loadImageFile() as a test to see if the file exists on-disk. There are a few ways to improve this:
- Add a helper function to simply check if the image URL exists on-disk when
self.embedExternalImagesisfalse - Call
loadImageFile()only whenself.embedExternalImagesistrue
| @@ -785,7 +790,8 @@ public class GLTFUnarchiver { | |||
| } | |||
| let glImage = images[index] | |||
|
|
|||
| var image: Image? | |||
| // An Image or a URL | |||
| var image: Any? | |||
There was a problem hiding this comment.
Same comment regarding Any usage as the above comments (first comment, second comment).
During development on an internal tool, a colleague and I needed an option to suppress GLTFSceneKit’s default behavior of embedding external image data into a SceneKit file.
This PR achieves that by adding a
embedExternalImagesoption toGTFSceneSourceconstructor, which istrueby default, thereby matching GLTFSceneKit’s current behavior.When the setting is
false, aGLTFUnarchiverclass instance will return aURLto an image, as extracted from a.gltffile, rather thanImagedata.