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

Crashes with texture_format: wgpu::TextureFormat::Rgba16Float #109

Open
julcst opened this issue Aug 2, 2024 · 2 comments
Open

Crashes with texture_format: wgpu::TextureFormat::Rgba16Float #109

julcst opened this issue Aug 2, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@julcst
Copy link

julcst commented Aug 2, 2024

Description

When building a RendererConfig with texture_format: wgpu::TextureFormat::Rgba16Float (to output to HDR):

let renderer_config = imgui_wgpu::RendererConfig {
    texture_format: wgpu::TextureFormat::Rgba16Float,
    ..Default::default()
};

wgpu panics:

ERROR wgpu::backend::wgpu_core > Handling wgpu errors as fatal by default
thread 'main' panicked at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:2996:5:
wgpu error: Validation Error

Caused by:
    In Queue::write_texture
    Number of bytes per row is less than the number of bytes in a complete row


stack backtrace:
   0: rust_begin_unwind
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/std/src/panicking.rs:652:5
   1: core::panicking::panic_fmt
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/panicking.rs:72:14
   2: wgpu::backend::wgpu_core::default_error_handler
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:2996:5
   3: core::ops::function::Fn::call
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/core/src/ops/function.rs:79:5
   4: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
             at /rustc/051478957371ee0084a7c0913941d2a8c4757bb9/library/alloc/src/boxed.rs:2077:9
   5: wgpu::backend::wgpu_core::ErrorSinkRaw::handle_error
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:2982:17
   6: wgpu::backend::wgpu_core::ContextWgpuCore::handle_error
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:293:9
   7: wgpu::backend::wgpu_core::ContextWgpuCore::handle_error_nolabel
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:305:9
   8: <wgpu::backend::wgpu_core::ContextWgpuCore as wgpu::context::Context>::queue_write_texture
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/backend/wgpu_core.rs:2221:17
   9: <T as wgpu::context::DynContext>::queue_write_texture
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/context.rs:2995:9
  10: wgpu::Queue::write_texture
             at /Users/???/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wgpu-0.20.1/src/lib.rs:4937:9
  11: imgui_wgpu::Texture::write
             at /Users/???/.cargo/git/checkouts/imgui-wgpu-rs-c1628ce4915bf3bb/bb0cbc2/src/lib.rs:220:9
  12: imgui_wgpu::Renderer::reload_font_texture
             at /Users/???/.cargo/git/checkouts/imgui-wgpu-rs-c1628ce4915bf3bb/bb0cbc2/src/lib.rs:791:9
  13: imgui_wgpu::Renderer::new
             at /Users/???/.cargo/git/checkouts/imgui-wgpu-rs-c1628ce4915bf3bb/bb0cbc2/src/lib.rs:515:9
...

The panic occurs in this method:

/// Write `data` to the texture.
///
/// - `data`: 32-bit RGBA bitmap data.
/// - `width`: The width of the source bitmap (`data`) in pixels.
/// - `height`: The height of the source bitmap (`data`) in pixels.
pub fn write(&self, queue: &Queue, data: &[u8], width: u32, height: u32) {
    queue.write_texture(
        // destination (sub)texture
        ImageCopyTexture {
            texture: &self.texture,
            mip_level: 0,
            origin: Origin3d { x: 0, y: 0, z: 0 },
            aspect: TextureAspect::All,
        },
        // source bitmap data
        data,
        // layout of the source bitmap
        ImageDataLayout {
            offset: 0,
            bytes_per_row: Some(width * 4),
            rows_per_image: Some(height),
        },
        // size of the source bitmap
        Extent3d {
            width,
            height,
            depth_or_array_layers: 1,
        },
    );
}

I am using:

imgui = "0.12.0"
imgui-wgpu = "0.24.0"
wgpu = "0.20.1"
@julcst julcst added the bug Something isn't working label Aug 2, 2024
@julcst

This comment was marked as outdated.

@julcst
Copy link
Author

julcst commented Aug 3, 2024

Ok instead of just converting the data I made the font atlas texture and surface texture different, this solution is way cleaner:

diff --git a/src/lib.rs b/src/lib.rs
index 992856c..1757e61 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -277,6 +277,7 @@ impl Texture {
 pub struct RendererConfig<'s> {
     pub texture_format: TextureFormat,
     pub depth_format: Option<TextureFormat>,
+    pub font_atlas_format: Option<TextureFormat>,
     pub sample_count: u32,
     pub shader: Option<ShaderModuleDescriptor<'s>>,
     pub vertex_shader_entry_point: Option<&'s str>,
@@ -289,6 +290,7 @@ impl<'s> RendererConfig<'s> {
         RendererConfig {
             texture_format: TextureFormat::Rgba8Unorm,
             depth_format: None,
+            font_atlas_format: None,
             sample_count: 1,
             shader: Some(shader),
             vertex_shader_entry_point: Some(VS_ENTRY_POINT),
@@ -362,6 +364,7 @@ impl Renderer {
         let RendererConfig {
             texture_format,
             depth_format,
+            font_atlas_format,
             sample_count,
             shader,
             vertex_shader_entry_point,
@@ -505,6 +508,7 @@ impl Renderer {
             config: RendererConfig {
                 texture_format,
                 depth_format,
+                font_atlas_format,
                 sample_count,
                 shader: None,
                 vertex_shader_entry_point: None,
@@ -785,6 +789,7 @@ impl Renderer {
                 height: handle.height,
                 ..Default::default()
             },
+            format: self.config.font_atlas_format,
             ..Default::default()
         };

In this fork I have a working version and also updated wgpu to 22.1.0: https://github.com/julcst/imgui-wgpu-rs.git.
I wondered though currently the default for the Texture::format is renderer.config.texture_format, wouldn't it be better if the defaults were simply wgpu::TextureFormat::Bgra8UnormSrgb and vec![wgpu::TextureFormat::Bgra8Unorm] because this is what almost everyone will want:

/// Create a new GPU texture width the specified `config`.
pub fn new(device: &Device, renderer: &Renderer, config: TextureConfig) -> Self {
    // Create the wgpu texture.
    let texture = Arc::new(device.create_texture(&TextureDescriptor {
        label: config.label,
        size: config.size,
        mip_level_count: config.mip_level_count,
        sample_count: config.sample_count,
        dimension: config.dimension,
        format: config.format.unwrap_or(renderer.config.texture_format),
        usage: config.usage,
        view_formats: &[config.format.unwrap_or(renderer.config.texture_format)],
    }));
    ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant