Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Glb textures should use bevy_render to load images #1454

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion crates/bevy_gltf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
33 changes: 8 additions & 25 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use bevy_render::{
prelude::{Color, Texture},
render_graph::base,
texture::{
AddressMode, Extent3d, FilterMode, SamplerDescriptor, TextureDimension, TextureFormat,
buffer_to_texture, AddressMode, FilterMode, ImageType, SamplerDescriptor, TextureError,
},
};
use bevy_scene::Scene;
Expand All @@ -26,7 +26,6 @@ use gltf::{
texture::{MagFilter, MinFilter, WrappingMode},
Material, Primitive,
};
use image::{GenericImageView, ImageFormat};
use std::{collections::HashMap, path::Path};
use thiserror::Error;

Expand All @@ -50,7 +49,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),
}
Expand Down Expand Up @@ -193,31 +192,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 = buffer_to_texture(buffer, ImageType::MimeType(mime_type))?;
texture.sampler = texture_sampler(&gltf_texture)?;
load_context.set_labeled_asset::<Texture>(&texture_label, LoadedAsset::new(texture));
}
}

Expand Down
96 changes: 73 additions & 23 deletions crates/bevy_render/src/texture/image_texture_loader.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use super::image_texture_conversion::image_to_texture;
use super::{image_texture_conversion::image_to_texture, Texture};
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)]
Expand All @@ -16,30 +17,17 @@ 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 = buffer_to_texture(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(())
})
}
Expand All @@ -49,6 +37,68 @@ 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
)
}
}

/// 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),
}

/// Load a bytes buffer in a [`Texture`], according to type `image_type`, using the `image` crate`
pub fn buffer_to_texture(buffer: &[u8], image_type: ImageType) -> Result<Texture, TextureError> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than being a floating function, maybe this should be added to Texture? Ex:

Texture::from_buffer(buffer: &[u8], image_type: ImageType) -> Result<Texture, TextureError> { ... }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok for me!

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))
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
14 changes: 0 additions & 14 deletions crates/bevy_render/src/texture/mod.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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::*;
Expand Down