From 1fcafc42108920691efccd9e15352261d07fffd0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois?= Date: Wed, 3 Mar 2021 21:36:16 +0000 Subject: [PATCH] Glb textures should use bevy_render to load images (#1454) Fixes #1396 Screenshot 2021-02-16 at 02 24 01 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. Screenshot 2021-02-16 at 02 44 56 --- crates/bevy_gltf/Cargo.toml | 1 - crates/bevy_gltf/src/loader.rs | 35 +++---------- .../src/texture/image_texture_loader.rs | 49 +++++++++-------- crates/bevy_render/src/texture/mod.rs | 14 ----- crates/bevy_render/src/texture/texture.rs | 52 ++++++++++++++++++- 5 files changed, 86 insertions(+), 65 deletions(-) diff --git a/crates/bevy_gltf/Cargo.toml b/crates/bevy_gltf/Cargo.toml index c0ef7d52ca..5e8d1ad72b 100644 --- a/crates/bevy_gltf/Cargo.toml +++ b/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" diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 68a84fb02f..941b651209 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/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_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_label, LoadedAsset::new(texture)); } } diff --git a/crates/bevy_render/src/texture/image_texture_loader.rs b/crates/bevy_render/src/texture/image_texture_loader.rs index 7b7c824f7d..1525c6330e 100644 --- a/crates/bevy_render/src/texture/image_texture_loader.rs +++ b/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(); + let dyn_img = + Texture::from_buffer(bytes, ImageType::Extension(ext)).map_err(|err| { + FileTextureError { + error: err, + path: format!("{}", load_context.path().display()), + } + })?; - // 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))); + 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::*; diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index d943dad2c5..0116754b7f 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/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::*; diff --git a/crates/bevy_render/src/texture/texture.rs b/crates/bevy_render/src/texture/texture.rs index a7536f7dc5..20939fc068 100644 --- a/crates/bevy_render/src/texture/texture.rs +++ b/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 { + 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> { @@ -245,3 +276,22 @@ impl RenderResource for Handle { 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), +}