Fix wrong rendering of some texture formats in light mode (#11225)

Author: emilk

crates/viewer/re_renderer/shader/rectangle.wgsl

@@ -18,6 +18,23 @@ const COLOR_MAPPER_TEXTURE       = 4u;
 const FILTER_NEAREST  = 1u;
 const FILTER_BILINEAR = 2u;
 
+// ----------------------------------------------------------------------------
+// See enum TextureAlpha
+
+/// Ignore the alpha channel and render the image opaquely.
+///
+/// Use this for textures that don't have an alpha channel.
+const TEXTURE_ALPHA_OPAQUE = 1u;
+
+/// The alpha in the texture is separate/unmulitplied.
+///
+/// Use this for images with separate (unmulitplied) alpha channels (the normal kind).
+const TEXTURE_ALPHA_SEPARATE_ALPHA = 2u;
+
+/// The RGB values have already been premultiplied with the alpha.
+const TEXTURE_ALPHA_ALREADY_PREMULTIPLIED = 3u;
+// ----------------------------------------------------------------------------
+
 struct UniformBuffer {
     /// Top left corner position in world space.
     top_left_corner_position: vec3f,
@@ -57,8 +74,8 @@ struct UniformBuffer {
     /// Boolean: decode 0-1 sRGB gamma to linear space before filtering?
     decode_srgb: u32,
 
-    /// Boolean: multiply RGB with alpha before filtering
-    multiply_rgb_with_alpha: u32,
+    /// TEXTURE_ALPHA_…
+    texture_alpha: u32,
 
     /// Boolean: swizzle RGBA to BGRA
     bgra_to_rgba: u32,

crates/viewer/re_renderer/shader/rectangle_fs.wgsl

@@ -37,9 +37,15 @@ fn decode_color(sampled_value: vec4f) -> vec4f {
         }
     }
 
-    // Premultiply alpha.
-    if rect_info.multiply_rgb_with_alpha != 0u {
+    if rect_info.texture_alpha == TEXTURE_ALPHA_OPAQUE {
+        rgba.a = 1.0; // ignore the alpha in the texture
+    } else if rect_info.texture_alpha == TEXTURE_ALPHA_SEPARATE_ALPHA {
+        // Premultiply alpha.
         rgba = vec4f(rgba.xyz * rgba.a, rgba.a);
+    } else if rect_info.texture_alpha == TEXTURE_ALPHA_ALREADY_PREMULTIPLIED {
+        // All good
+    } else {
+        rgba = ERROR_RGBA; // unknown enum
     }
 
     return rgba;

crates/viewer/re_renderer/src/renderer/mod.rs

@@ -19,7 +19,7 @@ pub use point_cloud::{
 };
 pub use rectangles::{
     ColorMapper, ColormappedTexture, RectangleDrawData, RectangleOptions, ShaderDecoding,
-    TextureFilterMag, TextureFilterMin, TexturedRect,
+    TextureAlpha, TextureFilterMag, TextureFilterMin, TexturedRect,
 };
 pub use test_triangle::TestTriangleDrawData;
 pub use world_grid::{WorldGridConfiguration, WorldGridDrawData, WorldGridRenderer};

crates/viewer/re_renderer/src/renderer/rectangles.rs

@@ -55,8 +55,25 @@ pub enum ShaderDecoding {
     Bgr,
 }
 
+/// How is the alpha channel in the texture?
+#[derive(Clone, Copy, Debug)]
+pub enum TextureAlpha {
+    /// Ignore the alpha channel and render the image opaquely.
+    ///
+    /// Use this for textures that don't have an alpha channel.
+    Opaque = 1,
+
+    /// The alpha in the texture is separate/unmulitplied.
+    ///
+    /// Use this for images with separate (unmulitplied) alpha channels (the normal kind).
+    SeparateAlpha = 2,
+
+    /// The RGB values have already been premultiplied with the alpha.
+    AlreadyPremultiplied = 3,
+}
+
 /// Describes a texture and how to map it to a color.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct ColormappedTexture {
     pub texture: GpuTexture2D,
 
@@ -72,11 +89,8 @@ pub struct ColormappedTexture {
     /// Only applies to [`wgpu::TextureFormat::Rgba8Unorm`] and float textures.
     pub decode_srgb: bool,
 
-    /// Multiply color channels with the alpha channel before filtering?
-    ///
-    /// Set this to false for textures that don't have an alpha channel or are already pre-multiplied.
-    /// Applied after range normalization and srgb decoding, before filtering.
-    pub multiply_rgb_with_alpha: bool,
+    /// How to treat the alpha channel.
+    pub texture_alpha: TextureAlpha,
 
     /// Raise the normalized values to this power (before any color mapping).
     /// Acts like an inverse brightness.
@@ -137,7 +151,7 @@ impl ColormappedTexture {
             decode_srgb,
             range: [0.0, 1.0],
             gamma: 1.0,
-            multiply_rgb_with_alpha: true,
+            texture_alpha: TextureAlpha::SeparateAlpha,
             color_mapper: ColorMapper::OffRGB,
             shader_decoding: None,
         }
@@ -148,7 +162,7 @@ impl ColormappedTexture {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct TexturedRect {
     /// Top left corner position in world space.
     pub top_left_corner_position: glam::Vec3,
@@ -184,7 +198,7 @@ impl TexturedRect {
     }
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct RectangleOptions {
     pub texture_filter_magnification: TextureFilterMag,
     pub texture_filter_minification: TextureFilterMin,
@@ -282,7 +296,7 @@ mod gpu_data {
         magnification_filter: u32,
 
         decode_srgb: u32,
-        multiply_rgb_with_alpha: u32,
+        texture_alpha: u32,
         bgra_to_rgba: u32,
         _row_padding: [u32; 1],
 
@@ -311,7 +325,7 @@ mod gpu_data {
                 range,
                 gamma,
                 color_mapper,
-                multiply_rgb_with_alpha,
+                texture_alpha,
                 shader_decoding,
             } = colormapped_texture;
 
@@ -380,7 +394,7 @@ mod gpu_data {
                 minification_filter,
                 magnification_filter,
                 decode_srgb: *decode_srgb as _,
-                multiply_rgb_with_alpha: *multiply_rgb_with_alpha as _,
+                texture_alpha: *texture_alpha as _,
                 bgra_to_rgba: bgra_to_rgba as _,
                 _row_padding: Default::default(),
                 _end_padding: Default::default(),

crates/viewer/re_view_tensor/src/tensor_slice_to_gpu.rs

@@ -41,7 +41,7 @@ pub fn colormapped_texture(
         texture,
         range: colormap.value_range,
         decode_srgb: false,
-        multiply_rgb_with_alpha: false,
+        texture_alpha: re_renderer::renderer::TextureAlpha::Opaque,
         gamma: *gamma.0,
         color_mapper: re_renderer::renderer::ColorMapper::Function(colormap_to_re_renderer(
             colormap.colormap,

crates/viewer/re_viewer_context/src/gpu_bridge/colormap.rs

@@ -47,7 +47,7 @@ fn colormap_preview_ui(
         texture: horizontal_gradient,
         range: [0.0, 1.0],
         decode_srgb: false,
-        multiply_rgb_with_alpha: false,
+        texture_alpha: re_renderer::renderer::TextureAlpha::Opaque,
         gamma: 1.0,
         shader_decoding: None,
         color_mapper: re_renderer::renderer::ColorMapper::Function(colormap_to_re_renderer(

crates/viewer/re_viewer_context/src/gpu_bridge/image_to_gpu.rs

@@ -4,13 +4,14 @@ use std::borrow::Cow;
 
 use anyhow::Context as _;
 use egui::{Rangef, util::hash};
+use half::f16;
 use wgpu::TextureFormat;
 
 use re_renderer::{
     RenderContext,
     device_caps::DeviceCaps,
     pad_rgb_to_rgba,
-    renderer::{ColorMapper, ColormappedTexture, ShaderDecoding},
+    renderer::{ColorMapper, ColormappedTexture, ShaderDecoding, TextureAlpha},
     resource_managers::{
         ImageDataDesc, SourceImageDataFormat, YuvMatrixCoefficients, YuvPixelLayout, YuvRange,
     },
@@ -141,21 +142,25 @@ fn color_image_to_gpu(
         ColorMapper::OffRGB
     };
 
-    // Assume that the texture has a separate (non-pre-multiplied) alpha.
-    // TODO(wumpf): There should be a way to specify whether a texture uses pre-multiplied alpha or not.
-    let multiply_rgb_with_alpha = image_format.has_alpha();
+    let texture_alpha = if image_format.has_alpha() {
+        // Assume that the texture has a separate (non-pre-multiplied) alpha.
+        // TODO(wumpf): There should be a way to specify whether a texture uses pre-multiplied alpha or not.
+        TextureAlpha::SeparateAlpha
+    } else {
+        TextureAlpha::Opaque
+    };
 
     let gamma = 1.0;
 
     re_log::trace_once!(
-        "color_tensor_to_gpu {debug_name:?}, range: {range:?}, decode_srgb: {decode_srgb:?}, multiply_rgb_with_alpha: {multiply_rgb_with_alpha:?}, gamma: {gamma:?}, color_mapper: {color_mapper:?}",
+        "color_tensor_to_gpu {debug_name:?}, range: {range:?}, decode_srgb: {decode_srgb:?}, texture_alpha: {texture_alpha:?}, gamma: {gamma:?}, color_mapper: {color_mapper:?}",
     );
 
     Ok(ColormappedTexture {
         texture: texture_handle,
         range: [range.min, range.max],
         decode_srgb,
-        multiply_rgb_with_alpha,
+        texture_alpha,
         gamma,
         color_mapper,
         shader_decoding,
@@ -327,7 +332,7 @@ pub fn texture_creation_desc_from_color_image<'a>(
             // Why not use `Rgba8UnormSrgb`? Because premul must happen _before_ sRGB decode, so we can't
             // use a "Srgb-aware" texture like `Rgba8UnormSrgb` for RGBA.
             (ColorModel::RGB, ChannelDatatype::U8) => (
-                pad_rgb_to_rgba(&image.buffer, u8::MAX).into(),
+                pad_rgb_to_rgba(&image.buffer, 0).into(),
                 SourceImageDataFormat::WgpuCompatible(TextureFormat::Rgba8Unorm),
             ),
             (ColorModel::RGBA, ChannelDatatype::U8) => (
@@ -349,7 +354,7 @@ pub fn texture_creation_desc_from_color_image<'a>(
             //
             // See also [`required_shader_decode`] which lists this case as a format that does not need to be decoded.
             (ColorModel::BGR, ChannelDatatype::U8) => {
-                let padded_data = pad_rgb_to_rgba(&image.buffer, u8::MAX).into();
+                let padded_data = pad_rgb_to_rgba(&image.buffer, 0).into();
                 let texture_format = if required_shader_decode(device_caps, &image.format).is_some()
                 {
                     TextureFormat::Rgba8Unorm
@@ -433,7 +438,7 @@ fn depth_image_to_gpu(
         texture,
         range: value_range,
         decode_srgb: false,
-        multiply_rgb_with_alpha: false,
+        texture_alpha: TextureAlpha::Opaque,
         gamma: 1.0,
         color_mapper: ColorMapper::Function(colormap_to_re_renderer(colormap)),
         shader_decoding: None,
@@ -509,7 +514,7 @@ fn segmentation_image_to_gpu(
         texture: main_texture_handle,
         range: [0.0, (colormap_width * colormap_height) as f32],
         decode_srgb: false, // Setting this to true would affect the class ids, not the color they resolve to.
-        multiply_rgb_with_alpha: false, // already premultiplied!
+        texture_alpha: TextureAlpha::AlreadyPremultiplied,
         gamma: 1.0,
         color_mapper: ColorMapper::Texture(colormap_texture_handle),
         shader_decoding: None,
@@ -562,37 +567,30 @@ fn general_texture_creation_desc_from_image<'a>(
         // BGR->RGB conversion is done in the shader.
         ColorModel::RGB | ColorModel::BGR => {
             // There are no 3-channel textures in wgpu, so we need to pad to 4 channels.
-            // What should we pad with? It depends on whether or not the shader interprets these as alpha.
-            // To be safe, we pad with the MAX value of integers, and with 1.0 for floats.
-            // TODO(emilk): tell the shader to ignore the alpha channel instead!
+            // What should we pad with?
+            // It doesn't matter - we tell the shader to ignore the alpha channel.
 
             match datatype {
-                ChannelDatatype::U8 => (
-                    pad_rgb_to_rgba(buf, u8::MAX).into(),
-                    TextureFormat::Rgba8Uint,
-                ),
-                ChannelDatatype::U16 => (pad_cast_img(image, u16::MAX), TextureFormat::Rgba16Uint),
-                ChannelDatatype::U32 => (pad_cast_img(image, u32::MAX), TextureFormat::Rgba32Uint),
+                ChannelDatatype::U8 => (pad_rgb_to_rgba(buf, 0).into(), TextureFormat::Rgba8Uint),
+                ChannelDatatype::U16 => (pad_cast_img::<u16>(image), TextureFormat::Rgba16Uint),
+                ChannelDatatype::U32 => (pad_cast_img::<u32>(image), TextureFormat::Rgba32Uint),
                 ChannelDatatype::U64 => (
-                    pad_and_narrow_and_cast(&image.to_slice(), 1.0, |x: u64| x as f32),
+                    pad_and_narrow_and_cast(&image.to_slice(), |x: u64| x as f32),
                     TextureFormat::Rgba32Float,
                 ),
 
-                ChannelDatatype::I8 => (pad_cast_img(image, i8::MAX), TextureFormat::Rgba8Sint),
-                ChannelDatatype::I16 => (pad_cast_img(image, i16::MAX), TextureFormat::Rgba16Sint),
-                ChannelDatatype::I32 => (pad_cast_img(image, i32::MAX), TextureFormat::Rgba32Sint),
+                ChannelDatatype::I8 => (pad_cast_img::<i8>(image), TextureFormat::Rgba8Sint),
+                ChannelDatatype::I16 => (pad_cast_img::<i16>(image), TextureFormat::Rgba16Sint),
+                ChannelDatatype::I32 => (pad_cast_img::<i32>(image), TextureFormat::Rgba32Sint),
                 ChannelDatatype::I64 => (
-                    pad_and_narrow_and_cast(&image.to_slice(), 1.0, |x: i64| x as f32),
+                    pad_and_narrow_and_cast(&image.to_slice(), |x: i64| x as f32),
                     TextureFormat::Rgba32Float,
                 ),
 
-                ChannelDatatype::F16 => (
-                    pad_cast_img(image, half::f16::from_f32(1.0)),
-                    TextureFormat::Rgba16Float,
-                ),
-                ChannelDatatype::F32 => (pad_cast_img(image, 1.0_f32), TextureFormat::Rgba32Float),
+                ChannelDatatype::F16 => (pad_cast_img::<f16>(image), TextureFormat::Rgba16Float),
+                ChannelDatatype::F32 => (pad_cast_img::<f32>(image), TextureFormat::Rgba32Float),
                 ChannelDatatype::F64 => (
-                    pad_and_narrow_and_cast(&image.to_slice(), 1.0, |x: f64| x as f32),
+                    pad_and_narrow_and_cast(&image.to_slice(), |x: f64| x as f32),
                     TextureFormat::Rgba32Float,
                 ),
             }
@@ -675,29 +673,30 @@ fn narrow_f64_to_f32s(slice: &[f64]) -> Cow<'static, [u8]> {
 }
 
 /// Pad an RGB image to RGBA and cast the results to bytes.
-fn pad_and_cast<T: Copy + bytemuck::Pod>(data: &[T], pad: T) -> Cow<'static, [u8]> {
+fn pad_and_cast<T: Copy + bytemuck::Pod + Default>(data: &[T]) -> Cow<'static, [u8]> {
     re_tracing::profile_function!();
     // TODO(emilk): optimize by combining the two steps into one; avoiding one allocation and memcpy
-    let padded: Vec<T> = pad_rgb_to_rgba(data, pad);
+    let padded: Vec<T> = pad_rgb_to_rgba(data, T::default());
     let bytes: Vec<u8> = bytemuck::pod_collect_to_vec(&padded);
     bytes.into()
 }
 
 /// Pad an RGB image to RGBA and cast the results to bytes.
-fn pad_cast_img<T: Copy + bytemuck::Pod>(img: &ImageInfo, pad: T) -> Cow<'static, [u8]> {
-    pad_and_cast(&img.to_slice(), pad)
+fn pad_cast_img<T: Copy + bytemuck::Pod + Default>(img: &ImageInfo) -> Cow<'static, [u8]> {
+    pad_and_cast::<T>(&img.to_slice())
 }
 
-fn pad_and_narrow_and_cast<T: Copy + bytemuck::Pod>(
+fn pad_and_narrow_and_cast<T: Copy + bytemuck::Pod + Default>(
     data: &[T],
-    pad: f32,
     narrow: impl Fn(T) -> f32,
 ) -> Cow<'static, [u8]> {
     re_tracing::profile_function!();
 
+    let alpha = 0.0; // The shader should just ignore it
+
     let floats: Vec<f32> = data
         .chunks_exact(3)
-        .flat_map(|chunk| [narrow(chunk[0]), narrow(chunk[1]), narrow(chunk[2]), pad])
+        .flat_map(|chunk| [narrow(chunk[0]), narrow(chunk[1]), narrow(chunk[2]), alpha])
         .collect();
     bytemuck::pod_collect_to_vec(&floats).into()
 }