raytracer: Use premultiplied alpha to describe surface color.

Part of fixing <#504>.
With this change, the raytracer now correctly renders blocks that
have light emission only (at least, in volumetric mode; the surface-only
mode is multiplying the emission by the number of voxels, which is
not really the right outcome).

It is incoherent or at least undesirable to multiply light emissions
by the reflectance color’s alpha. To avoid doing that, the cleanest
solution is to switch to premultiplied alpha, where the accumulator
only uses the alpha to determine the effect on future (more distant)
surfaces.

Conveniently, we already have a type that implements exactly this
representation: `ColorBuf`! Or, not exactly — the alpha is actually
complemented (1 − α), making it transmittance instead, but that’s
what we actually need.

This commit breaks the `icons` rendering test, but we need to fix
the same bug in `wgpu` rendering and also update the icon to match
the new paradigm, so it's going to be broken sometime unless I were to
squash all three changes into one commit, which I don't want to do.

CHANGELOG.md

@@ -17,6 +17,9 @@
     - `block::EvalBlockError` is now a `struct` with an inner `ErrorKind` enum, instead of an enum, and contains more information.
     - `block::Move`’s means of construction have been changed to be more systematic and orthogonal. In particular, paired moves are constructed from unpaired ones.
 
+- `all-is-cubes-render` library:
+    - The trait method `raytracer::Accumulate::add()` now accepts the surface color via a `ColorBuf` (which acts essentially as a form of premultiplied alpha) rather than `Rgba`. This enables more consistent handling of emissive materials. See the method documentation for details.
+
 ### Removed
 
 ## 0.8.0 (2024-07-08)

all-is-cubes-desktop/src/terminal.rs

@@ -363,18 +363,20 @@ impl Accumulate for ColorCharacterBuf {
     }
 
     #[inline]
-    fn add(&mut self, surface_color: Rgba, text: &Self::BlockData) {
+    fn add(&mut self, surface: ColorBuf, text: &Self::BlockData) {
         if self.override_color {
             return;
         }
 
-        self.color.add(surface_color, &());
-        self.text.add(surface_color, text);
+        self.color.add(surface, &());
+        self.text.add(surface, text);
     }
 
     fn hit_nothing(&mut self) {
-        self.text
-            .add(Rgba::TRANSPARENT, &CharacterRtData(literal_substr!(" ")));
+        self.text.add(
+            Rgba::TRANSPARENT.into(),
+            &CharacterRtData(literal_substr!(" ")),
+        );
         self.override_color = true;
     }
 

all-is-cubes-render/src/raytracer/ortho.rs

@@ -296,8 +296,8 @@ impl raytracer::Accumulate for OrthoBuf {
         self.color.opaque()
     }
 
-    fn add(&mut self, surface_color: Rgba, &resolution: &Self::BlockData) {
-        self.color.add(surface_color, &());
+    fn add(&mut self, surface: raytracer::ColorBuf, &resolution: &Self::BlockData) {
+        self.color.add(surface, &());
         self.max_resolution = self.max_resolution.max(resolution);
     }
 

all-is-cubes-render/src/raytracer/renderer.rs

@@ -652,7 +652,7 @@ mod tests {
             fn opaque(&self) -> bool {
                 self.custom_options.is_empty()
             }
-            fn add(&mut self, _: Rgba, block_data: &Self::BlockData) {
+            fn add(&mut self, _: ColorBuf, block_data: &Self::BlockData) {
                 if self.custom_options.is_empty() {
                     *self = *block_data;
                 }

all-is-cubes/src/raytracer.rs

@@ -518,7 +518,7 @@ impl<P: Accumulate> TracingState<P> {
             // Abort excessively long traces.
             self.accumulator = Default::default();
             self.accumulator
-                .add(Rgba::new(1.0, 1.0, 1.0, 1.0), &P::BlockData::error(options));
+                .add(Rgba::WHITE.into(), &P::BlockData::error(options));
             true
         } else {
             self.accumulator.opaque()
@@ -532,7 +532,7 @@ impl<P: Accumulate> TracingState<P> {
             self.accumulator.hit_nothing();
         }
 
-        self.accumulator.add(sky_color, sky_data);
+        self.accumulator.add(sky_color.into(), sky_data);
 
         // Debug visualization of number of raytracing steps.
         // TODO: Make this togglable and less of a kludge — we'd like to be able to mix with
@@ -541,7 +541,9 @@ impl<P: Accumulate> TracingState<P> {
         if DEBUG_STEPS && self.accumulator.opaque() {
             self.accumulator = Default::default();
             self.accumulator.add(
-                (rgb_const!(0.02, 0.002, 0.0) * self.cubes_traced as f32).with_alpha_one(),
+                (rgb_const!(0.02, 0.002, 0.0) * self.cubes_traced as f32)
+                    .with_alpha_one()
+                    .into(),
                 sky_data,
             );
         }
@@ -561,8 +563,8 @@ impl<P: Accumulate> TracingState<P> {
         surface: &Surface<'_, P::BlockData>,
         rt: &SpaceRaytracer<P::BlockData>,
     ) {
-        if let Some(color) = surface.to_lit_color(rt) {
-            self.accumulator.add(color, surface.block_data);
+        if let Some(light) = surface.to_light(rt) {
+            self.accumulator.add(light, surface.block_data);
         }
     }
 
@@ -629,8 +631,8 @@ pub(crate) fn trace_for_eval(
     let mut emission = Vector3D::zero();
 
     while let Some(voxel) = voxels.get(cube) {
-        emission += Vector3D::from(voxel.emission) * color_buf.ray_alpha;
-        color_buf.add(apply_transmittance(voxel.color, thickness), &());
+        emission += Vector3D::from(voxel.emission) * color_buf.transmittance;
+        color_buf.add(apply_transmittance(voxel.color, thickness).into(), &());
 
         if color_buf.opaque() {
             break;
@@ -700,7 +702,7 @@ mod tests {
             let modified_color = apply_transmittance(color, (count as f32).recip());
             let mut color_buf = ColorBuf::default();
             for _ in 0..count {
-                color_buf.add(modified_color, &());
+                color_buf.add(modified_color.into(), &());
             }
             let actual = Rgba::from(color_buf);
             let error: Vec<f32> = <[f32; 4]>::from(actual)

all-is-cubes/src/raytracer/accum.rs

@@ -62,14 +62,17 @@ pub trait Accumulate: Default {
     /// be affected by future calls to [`Self::add`].
     fn opaque(&self) -> bool;
 
-    /// Adds the color of a surface to the buffer. The provided color should already
-    /// have the effect of lighting applied.
+    /// Adds the light from a surface, and the opacity of that surface, to the accumulator.
+    /// This surface is positioned behind/beyond all previous `add()`ed surfaces.
     ///
-    /// You should probably give this method the `#[inline]` attribute.
+    /// The given [`ColorBuf`] represents the opacity and the camera-ward light output of the
+    /// encountered surface.
+    /// Implementations are responsible for reducing it according to the transmittance (inverse
+    /// opacity) of previously encountered surfaces which obscure it.
     ///
-    /// TODO: this interface might want even more information; generalize it to be
+    /// TODO: this interface might want even more information (e.g. depth); generalize it to be
     /// more future-proof.
-    fn add(&mut self, surface_color: Rgba, block_data: &Self::BlockData);
+    fn add(&mut self, surface: ColorBuf, block_data: &Self::BlockData);
 
     /// Called before the ray traverses any surfaces found in a block; that is,
     /// before all [`add()`](Self::add) calls pertaining to that block.
@@ -96,7 +99,7 @@ pub trait Accumulate: Default {
         let mut result = Self::default();
         // TODO: Should give RtBlockData a dedicated method for this, but we haven't
         // yet had a use case where it matters.
-        result.add(color, &Self::BlockData::sky(options));
+        result.add(color.into(), &Self::BlockData::sky(options));
         result
     }
 
@@ -157,6 +160,9 @@ impl RtBlockData for Resolution {
 
 /// Implements [`Accumulate`] for RGB(A) color with [`f32`] components,
 /// and conversion to [`Rgba`].
+///
+/// In addition to its use in constructing images, this type is also used as an intermediate
+/// format for surface colors presented to any other [`Accumulate`] implementation.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct ColorBuf {
     /// Color buffer.
@@ -166,11 +172,23 @@ pub struct ColorBuf {
     /// display supposing that everything not already traced is black.
     ///
     /// Note: Not using the [`Rgb`](crate::math::Rgb) type so as to skip NaN checks.
-    color_accumulator: Vector3D<f32, Intensity>,
+    /// This should never end up NaN, but we don't assert that property.
+    light: Vector3D<f32, Intensity>,
 
     /// Fraction of the color value that is to be determined by future, rather than past,
     /// tracing; starts at 1.0 and decreases as surfaces are encountered.
-    pub(super) ray_alpha: f32,
+    ///
+    /// The range of this value is between 1.0 and 0.0, inclusive.
+    pub(super) transmittance: f32,
+}
+
+impl ColorBuf {
+    pub(crate) fn from_light_and_transmittance(light: Rgb, transmittance: f32) -> ColorBuf {
+        Self {
+            light: light.into(),
+            transmittance,
+        }
+    }
 }
 
 impl Accumulate for ColorBuf {
@@ -180,27 +198,26 @@ impl Accumulate for ColorBuf {
     fn opaque(&self) -> bool {
         // Let's suppose that we don't care about differences that can't be represented
         // in 8-bit color...not considering gamma.
-        self.ray_alpha < 1.0 / 256.0
+        self.transmittance < 1.0 / 256.0
     }
 
     #[inline]
-    fn add(&mut self, surface_color: Rgba, _block_data: &Self::BlockData) {
-        let color_vector: Vector3D<f32, Intensity> = surface_color.to_rgb().into();
-        let surface_alpha = surface_color.alpha().into_inner();
-        let alpha_for_add = surface_alpha * self.ray_alpha;
-        self.ray_alpha *= 1.0 - surface_alpha;
-        self.color_accumulator += color_vector * alpha_for_add;
+    fn add(&mut self, surface: ColorBuf, _block_data: &Self::BlockData) {
+        // Note that the order of these assignments matters.
+        // surface.transmittance is only applied to surfaces *after* this one.
+        self.light += surface.light * self.transmittance;
+        self.transmittance *= surface.transmittance;
     }
 
     #[inline]
     fn mean<const N: usize>(items: [Self; N]) -> Self {
         Self {
-            color_accumulator: items
+            light: items
                 .iter()
-                .map(|cb| cb.color_accumulator)
+                .map(|cb| cb.light)
                 .sum::<Vector3D<f32, Intensity>>()
                 / (N as f32),
-            ray_alpha: items.iter().map(|cb| cb.ray_alpha).sum::<f32>() / (N as f32),
+            transmittance: items.iter().map(|cb| cb.transmittance).sum::<f32>() / (N as f32),
         }
     }
 }
@@ -209,8 +226,8 @@ impl Default for ColorBuf {
     #[inline]
     fn default() -> Self {
         Self {
-            color_accumulator: Vector3D::zero(),
-            ray_alpha: 1.0,
+            light: Vector3D::zero(),
+            transmittance: 1.0,
         }
     }
 }
@@ -221,26 +238,52 @@ impl From<ColorBuf> for Rgba {
     /// Not tone mapped; consider using [`Camera::post_process_color()`] for that.
     ///
     /// [`Camera::post_process_color()`]: crate::camera::Camera::post_process_color()
+    //---
+    // TODO: Converting arbitrary HDR+opacity colors to non-premultiplied RGBA is incorrect
+    // in the general case. We should allow for tone-mapping in premultiplied form, either by
+    // replacing this conversion with a custom method, or changing the [`Rgba`] type to be
+    // premultiplied.
     fn from(buf: ColorBuf) -> Rgba {
-        if buf.ray_alpha >= 1.0 {
+        if buf.transmittance >= 1.0 {
             // Special case to avoid dividing by zero
             Rgba::TRANSPARENT
         } else {
-            let color_alpha = 1.0 - buf.ray_alpha;
-            let non_premultiplied_color = buf.color_accumulator / color_alpha;
+            let color_alpha = 1.0 - buf.transmittance;
+            let non_premultiplied_color = buf.light / color_alpha;
             Rgb::try_from(non_premultiplied_color)
                 .unwrap_or_else(|_| rgb_const!(1.0, 0.0, 0.0))
                 .with_alpha(NotNan::new(color_alpha).unwrap_or(notnan!(1.0)))
         }
     }
 }
 
+impl From<Rgba> for ColorBuf {
+    /// Converts the given [`Rgba`] color value, interpreted as the reflectance and opacity of
+    /// a surface lit by white light with luminance 1.0, into a [`ColorBuf`].
+    fn from(value: Rgba) -> Self {
+        let alpha = value.alpha().into_inner();
+        Self {
+            light: Vector3D::new(
+                value.red().into_inner() * alpha,
+                value.green().into_inner() * alpha,
+                value.blue().into_inner() * alpha,
+            ),
+            transmittance: 1.0 - alpha,
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
     fn color_buf() {
+        // TODO: This entire test should be revisited with our new understanding about
+        // premultiplied alpha’s role in color accumulation.
+        // Maybe we can fix the failing assertions.
+        // <https://github.com/kpreid/all-is-cubes/issues/504>
+
         let color_1 = Rgba::new(1.0, 0.0, 0.0, 0.75);
         let color_2 = Rgba::new(0.0, 1.0, 0.0, 0.5);
         let color_3 = Rgba::new(0.0, 0.0, 1.0, 1.0);
@@ -249,11 +292,11 @@ mod tests {
         assert_eq!(Rgba::from(buf), Rgba::TRANSPARENT);
         assert!(!buf.opaque());
 
-        buf.add(color_1, &());
+        buf.add(color_1.into(), &());
         assert_eq!(Rgba::from(buf), color_1);
         assert!(!buf.opaque());
 
-        buf.add(color_2, &());
+        buf.add(color_2.into(), &());
         // TODO: this is not the right assertion because it's the premultiplied form.
         // assert_eq!(
         //     buf.result(),
@@ -262,7 +305,7 @@ mod tests {
         // );
         assert!(!buf.opaque());
 
-        buf.add(color_3, &());
+        buf.add(color_3.into(), &());
         assert!(Rgba::from(buf).fully_opaque());
         //assert_eq!(
         //    buf.result(),

all-is-cubes/src/raytracer/surface.rs

@@ -4,7 +4,7 @@ use crate::block::Evoxel;
 use crate::camera::LightingOption;
 use crate::math::{Cube, Face7, FaceMap, FreeCoordinate, FreePoint, Rgb, Rgba, Vol};
 use crate::raycast::{RayIsh as _, RaycasterIsh};
-use crate::raytracer::{RtBlockData, SpaceRaytracer, TracingBlock, TracingCubeData};
+use crate::raytracer::{ColorBuf, RtBlockData, SpaceRaytracer, TracingBlock, TracingCubeData};
 
 /// Description of a surface the ray passes through (or from the volumetric perspective,
 /// a transition from one material to another).
@@ -44,13 +44,17 @@ impl<D: RtBlockData> Surface<'_, D> {
         !diffuse_color.fully_transparent() || emission != Rgb::ZERO
     }
 
-    /// Convert the surface and its lighting to a single RGBA value as determined by
-    /// the given graphics options, or [`None`] if it is invisible.
+    /// Combine the surface properties, lighting, and graphics options to produce
+    /// a light intensity and a transmittance value for light arriving from behind the surface;
+    /// or [`None`] if it is invisible.
+    ///
+    /// Note that the result is “premultiplied alpha”; the returned color should *not*
+    /// be modified in any way by the returned transmittance.
     ///
     /// Note that this is completely unaware of volume/thickness; that is handled by
     /// `TracingState::trace_through_span()` tweaking the data before this is called.
     #[inline]
-    pub(crate) fn to_lit_color(&self, rt: &SpaceRaytracer<D>) -> Option<Rgba> {
+    pub(crate) fn to_light(&self, rt: &SpaceRaytracer<D>) -> Option<ColorBuf> {
         let diffuse_color = rt
             .graphics_options
             .transparency
@@ -62,9 +66,13 @@ impl<D: RtBlockData> Surface<'_, D> {
 
         let illumination = self.compute_illumination(rt);
         // Combine reflected and emitted light to produce the outgoing light.
-        let outgoing_rgb = diffuse_color.to_rgb() * illumination + self.emission;
+        let outgoing_rgb =
+            diffuse_color.to_rgb() * illumination * diffuse_color.alpha() + self.emission;
 
-        Some(outgoing_rgb.with_alpha(diffuse_color.alpha()))
+        Some(ColorBuf::from_light_and_transmittance(
+            outgoing_rgb,
+            1.0 - diffuse_color.alpha().into_inner(),
+        ))
     }
 
     fn compute_illumination(&self, rt: &SpaceRaytracer<D>) -> Rgb {

all-is-cubes/src/raytracer/text.rs

@@ -10,7 +10,7 @@ use euclid::size2;
 use unicode_segmentation::UnicodeSegmentation;
 
 use crate::camera::{eye_for_look_at, Camera, GraphicsOptions, Viewport};
-use crate::math::{FreeVector, Rgba};
+use crate::math::FreeVector;
 use crate::raytracer::{Accumulate, RtBlockData, RtOptionsRef, SpaceRaytracer};
 use crate::space::{Space, SpaceBlockData};
 
@@ -59,7 +59,7 @@ impl Accumulate for CharacterBuf {
     }
 
     #[inline]
-    fn add(&mut self, _surface_color: Rgba, d: &Self::BlockData) {
+    fn add(&mut self, _surface: super::ColorBuf, d: &Self::BlockData) {
         if self.hit_text.is_none() {
             self.hit_text = Some(d.0.clone());
         }
@@ -148,6 +148,7 @@ mod tests {
     use crate::block::{Block, Resolution::R4};
     use crate::color_block;
     use crate::content::make_some_blocks;
+    use crate::math::Rgba;
     use crate::universe::Universe;
     use euclid::vec3;
     use std::string::ToString;