diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ef7b5fd2..18313f4dd 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/all-is-cubes-desktop/src/terminal.rs b/all-is-cubes-desktop/src/terminal.rs index 073935ba8..8b28acee7 100644 --- a/all-is-cubes-desktop/src/terminal.rs +++ b/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; } diff --git a/all-is-cubes-render/src/raytracer/ortho.rs b/all-is-cubes-render/src/raytracer/ortho.rs index aa077b7fa..0b5a4a2ee 100644 --- a/all-is-cubes-render/src/raytracer/ortho.rs +++ b/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); } diff --git a/all-is-cubes-render/src/raytracer/renderer.rs b/all-is-cubes-render/src/raytracer/renderer.rs index d1ab30e57..96e910070 100644 --- a/all-is-cubes-render/src/raytracer/renderer.rs +++ b/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; } diff --git a/all-is-cubes/src/raytracer.rs b/all-is-cubes/src/raytracer.rs index 11159efad..08ed1d6ed 100644 --- a/all-is-cubes/src/raytracer.rs +++ b/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) diff --git a/all-is-cubes/src/raytracer/accum.rs b/all-is-cubes/src/raytracer/accum.rs index 8f25998b9..70783faf7 100644 --- a/all-is-cubes/src/raytracer/accum.rs +++ b/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,13 +238,18 @@ 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))) @@ -235,12 +257,33 @@ impl From<ColorBuf> for Rgba { } } +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(), diff --git a/all-is-cubes/src/raytracer/surface.rs b/all-is-cubes/src/raytracer/surface.rs index edac0674f..946b3bf38 100644 --- a/all-is-cubes/src/raytracer/surface.rs +++ b/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 { diff --git a/all-is-cubes/src/raytracer/text.rs b/all-is-cubes/src/raytracer/text.rs index 97368e090..4c4d29206 100644 --- a/all-is-cubes/src/raytracer/text.rs +++ b/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; diff --git a/test-renderers/expected/renderers/emission_only-surf-all.png b/test-renderers/expected/renderers/emission_only-surf-all.png new file mode 100644 index 000000000..ce4bb30f9 Binary files /dev/null and b/test-renderers/expected/renderers/emission_only-surf-all.png differ diff --git a/test-renderers/expected/renderers/emission_only-vol-all.png b/test-renderers/expected/renderers/emission_only-vol-all.png new file mode 100644 index 000000000..2e8f2b522 Binary files /dev/null and b/test-renderers/expected/renderers/emission_only-vol-all.png differ