Glb textures should use bevy_render to load images (#1454)

Fixes #1396 

<img width="1392" alt="Screenshot 2021-02-16 at 02 24 01" src="https://user-images.githubusercontent.com/8672791/108011774-1b991a80-7008-11eb-979e-6ebfc51fba3c.png">

Issue was that, when loading an image directly from its bytes in the binary glb file, it didn't follow the same flow as when loaded as a texture file. This PR removes the dependency to `image` from `bevy_gltf`, and load the image using `bevy_render` in all cases. I also added support for more mime types while there.

<img width="1392" alt="Screenshot 2021-02-16 at 02 44 56" src="https://user-images.githubusercontent.com/8672791/108011915-674bc400-7008-11eb-83d4-ded96a38919b.png">

Author: mockersf

crates/bevy_gltf/Cargo.toml

@@ -27,7 +27,6 @@ bevy_scene = { path = "../bevy_scene", version = "0.4.0" }
 
 # other
 gltf = { version = "0.15.2", default-features = false, features = ["utils", "names", "KHR_materials_unlit"] }
-image = { version = "0.23.12", default-features = false }
 thiserror = "1.0"
 anyhow = "1.0"
 base64 = "0.13.0"

crates/bevy_gltf/src/loader.rs

@@ -12,9 +12,7 @@ use bevy_render::{
     pipeline::PrimitiveTopology,
     prelude::{Color, Texture},
     render_graph::base,
-    texture::{
-        AddressMode, Extent3d, FilterMode, SamplerDescriptor, TextureDimension, TextureFormat,
-    },
+    texture::{AddressMode, FilterMode, ImageType, SamplerDescriptor, TextureError},
 };
 use bevy_scene::Scene;
 use bevy_transform::{
@@ -26,7 +24,6 @@ use gltf::{
     texture::{MagFilter, MinFilter, WrappingMode},
     Material, Primitive,
 };
-use image::{GenericImageView, ImageFormat};
 use std::{collections::HashMap, path::Path};
 use thiserror::Error;
 
@@ -50,7 +47,7 @@ pub enum GltfError {
     #[error("invalid image mime type")]
     InvalidImageMimeType(String),
     #[error("failed to load an image")]
-    ImageError(#[from] image::ImageError),
+    ImageError(#[from] TextureError),
     #[error("failed to load an asset path")]
     AssetIoError(#[from] AssetIoError),
 }
@@ -193,31 +190,15 @@ async fn load_gltf<'a, 'b>(
         })
         .collect();
 
-    for texture in gltf.textures() {
-        if let gltf::image::Source::View { view, mime_type } = texture.source().source() {
+    for gltf_texture in gltf.textures() {
+        if let gltf::image::Source::View { view, mime_type } = gltf_texture.source().source() {
             let start = view.offset() as usize;
             let end = (view.offset() + view.length()) as usize;
             let buffer = &buffer_data[view.buffer().index()][start..end];
-            let format = match mime_type {
-                "image/png" => Ok(ImageFormat::Png),
-                "image/jpeg" => Ok(ImageFormat::Jpeg),
-                _ => Err(GltfError::InvalidImageMimeType(mime_type.to_string())),
-            }?;
-            let image = image::load_from_memory_with_format(buffer, format)?;
-            let size = image.dimensions();
-            let image = image.into_rgba8();
-
-            let texture_label = texture_label(&texture);
-            load_context.set_labeled_asset::<Texture>(
-                &texture_label,
-                LoadedAsset::new(Texture {
-                    data: image.clone().into_vec(),
-                    size: Extent3d::new(size.0, size.1, 1),
-                    dimension: TextureDimension::D2,
-                    format: TextureFormat::Rgba8Unorm,
-                    sampler: texture_sampler(&texture)?,
-                }),
-            );
+            let texture_label = texture_label(&gltf_texture);
+            let mut texture = Texture::from_buffer(buffer, ImageType::MimeType(mime_type))?;
+            texture.sampler = texture_sampler(&gltf_texture)?;
+            load_context.set_labeled_asset::<Texture>(&texture_label, LoadedAsset::new(texture));
         }
     }
 

crates/bevy_render/src/texture/image_texture_loader.rs

@@ -1,7 +1,8 @@
-use super::image_texture_conversion::image_to_texture;
+use super::texture::{ImageType, Texture, TextureError};
 use anyhow::Result;
 use bevy_asset::{AssetLoader, LoadContext, LoadedAsset};
 use bevy_utils::BoxedFuture;
+use thiserror::Error;
 
 /// Loader for images that can be read by the `image` crate.
 #[derive(Clone, Default)]
@@ -16,30 +17,18 @@ impl AssetLoader for ImageTextureLoader {
         load_context: &'a mut LoadContext,
     ) -> BoxedFuture<'a, Result<()>> {
         Box::pin(async move {
-            // Find the image type we expect. A file with the extension "png" should
-            // probably load as a PNG.
-
+            // use the file extension for the image type
             let ext = load_context.path().extension().unwrap().to_str().unwrap();
 
-            let img_format = image::ImageFormat::from_extension(ext)
-                .ok_or_else(|| {
-                    format!(
-                    "Unexpected image format {:?} for file {}, this is an error in `bevy_render`.",
-                    ext,
-                    load_context.path().display()
-                )
-                })
-                .unwrap();
-
-            // Load the image in the expected format.
-            // Some formats like PNG allow for R or RG textures too, so the texture
-            // format needs to be determined. For RGB textures an alpha channel
-            // needs to be added, so the image data needs to be converted in those
-            // cases.
-
-            let dyn_img = image::load_from_memory_with_format(bytes, img_format)?;
-
-            load_context.set_default_asset(LoadedAsset::new(image_to_texture(dyn_img)));
+            let dyn_img =
+                Texture::from_buffer(bytes, ImageType::Extension(ext)).map_err(|err| {
+                    FileTextureError {
+                        error: err,
+                        path: format!("{}", load_context.path().display()),
+                    }
+                })?;
+
+            load_context.set_default_asset(LoadedAsset::new(dyn_img));
             Ok(())
         })
     }
@@ -49,6 +38,22 @@ impl AssetLoader for ImageTextureLoader {
     }
 }
 
+/// An error that occurs when loading a texture from a file
+#[derive(Error, Debug)]
+pub struct FileTextureError {
+    error: TextureError,
+    path: String,
+}
+impl std::fmt::Display for FileTextureError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::result::Result<(), std::fmt::Error> {
+        write!(
+            f,
+            "Error reading image file {}: {}, this is an error in `bevy_render`.",
+            self.path, self.error
+        )
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/bevy_render/src/texture/mod.rs

@@ -1,12 +1,5 @@
 #[cfg(feature = "hdr")]
 mod hdr_texture_loader;
-#[cfg(any(
-    feature = "png",
-    feature = "dds",
-    feature = "tga",
-    feature = "jpeg",
-    feature = "bmp"
-))]
 mod image_texture_loader;
 mod sampler_descriptor;
 #[allow(clippy::module_inception)]
@@ -18,13 +11,6 @@ pub(crate) mod image_texture_conversion;
 
 #[cfg(feature = "hdr")]
 pub use hdr_texture_loader::*;
-#[cfg(any(
-    feature = "png",
-    feature = "dds",
-    feature = "tga",
-    feature = "jpeg",
-    feature = "bmp"
-))]
 pub use image_texture_loader::*;
 pub use sampler_descriptor::*;
 pub use texture::*;

crates/bevy_render/src/texture/texture.rs

@@ -1,4 +1,7 @@
-use super::{Extent3d, SamplerDescriptor, TextureDescriptor, TextureDimension, TextureFormat};
+use super::{
+    image_texture_conversion::image_to_texture, Extent3d, SamplerDescriptor, TextureDescriptor,
+    TextureDimension, TextureFormat,
+};
 use crate::renderer::{
     RenderResource, RenderResourceContext, RenderResourceId, RenderResourceType,
 };
@@ -7,6 +10,7 @@ use bevy_asset::{AssetEvent, Assets, Handle};
 use bevy_ecs::Res;
 use bevy_reflect::TypeUuid;
 use bevy_utils::HashSet;
+use thiserror::Error;
 
 pub const TEXTURE_ASSET_INDEX: u64 = 0;
 pub const SAMPLER_ASSET_INDEX: u64 = 1;
@@ -212,6 +216,33 @@ impl Texture {
             render_resource_context.remove_asset_resource(handle, SAMPLER_ASSET_INDEX);
         }
     }
+
+    /// Load a bytes buffer in a [`Texture`], according to type `image_type`, using the `image` crate`
+    pub fn from_buffer(buffer: &[u8], image_type: ImageType) -> Result<Texture, TextureError> {
+        let format = match image_type {
+            ImageType::MimeType(mime_type) => match mime_type {
+                "image/png" => Ok(image::ImageFormat::Png),
+                "image/vnd-ms.dds" => Ok(image::ImageFormat::Dds),
+                "image/x-targa" => Ok(image::ImageFormat::Tga),
+                "image/x-tga" => Ok(image::ImageFormat::Tga),
+                "image/jpeg" => Ok(image::ImageFormat::Jpeg),
+                "image/bmp" => Ok(image::ImageFormat::Bmp),
+                "image/x-bmp" => Ok(image::ImageFormat::Bmp),
+                _ => Err(TextureError::InvalidImageMimeType(mime_type.to_string())),
+            },
+            ImageType::Extension(extension) => image::ImageFormat::from_extension(extension)
+                .ok_or_else(|| TextureError::InvalidImageMimeType(extension.to_string())),
+        }?;
+
+        // Load the image in the expected format.
+        // Some formats like PNG allow for R or RG textures too, so the texture
+        // format needs to be determined. For RGB textures an alpha channel
+        // needs to be added, so the image data needs to be converted in those
+        // cases.
+
+        let dyn_img = image::load_from_memory_with_format(buffer, format)?;
+        Ok(image_to_texture(dyn_img))
+    }
 }
 
 impl RenderResource for Option<Handle<Texture>> {
@@ -245,3 +276,22 @@ impl RenderResource for Handle<Texture> {
         Some(self)
     }
 }
+
+/// An error that occurs when loading a texture
+#[derive(Error, Debug)]
+pub enum TextureError {
+    #[error("invalid image mime type")]
+    InvalidImageMimeType(String),
+    #[error("invalid image extension")]
+    InvalidImageExtension(String),
+    #[error("failed to load an image")]
+    ImageError(#[from] image::ImageError),
+}
+
+/// Type of a raw image buffer
+pub enum ImageType<'a> {
+    /// Mime type of an image, for example `"image/png"`
+    MimeType(&'a str),
+    /// Extension of an image file, for example `"png"`
+    Extension(&'a str),
+}