Skip to content

Commit

Permalink
Light: Round to nearest instead of zero in PackedLight.
Browse files Browse the repository at this point in the history
This reduces the size of the largest rounding error. Of course, light
calculation inevitably produces lots of rounding cases, so all the
test files change. Also, something is funny in the tone-mapping tests
(see the updated comment there).
kpreid committed Jan 22, 2025
1 parent 4d67212 commit 1e68913
Showing 21 changed files with 53 additions and 30 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -14,22 +14,22 @@
 
 
 ▄ ▄ ▄      ▄ ▄ ▄ 
▄ ▄ ▄▄▄▄▄  ▄▄▄▄ ▄▄▄▄ ▄▄ ▄ ▄ ▄
 ▄ ▄ ▄▄ ▄ ▄ ▄▄ ▄▄▄▄  ▄▄▄▄▄ ▄▄ 
  ▄▄   ▄ ▄▄   ▄  
▄▄▄     
 ▄▄ ▄ ▄▄ ▄  ▄ ▄ ▄ ▄▄▄▄▄▄▄▄ ▄
  ▄▄▄▄▄▄▄ ▄ ▄▄  ▄  
▄ ▄▄ ▄ ▄▄▄  ▄    ▄ 
 ▄▄  ▄ ▄▄ ▄▄ ▄▄ ▄ ▄  
 ▄▄ ▄▄▄▄▄▄▄ 
 
 
 
 
 
 
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
▄ ▄ ▄▄▄▄▄  ▄▄▄▄ ▄▄▄▄ ▄▄▄▄▄▄ ▄
 ▄ ▄ ▄ ▄ ▄ ▄▄ ▄▄▄▄  ▄▄▄▄▄▄ ▄▄ 
  ▄▄   ▄ ▄▄   ▄  
     
 ▄ ▄ ▄▄ ▄   ▄ ▄ ▄ ▄▄▄▄▄▄▄▄ ▄
  ▄▄▄▄▄▄▄ ▄ ▄▄  ▄  
▄▄ ▄▄ ▄▄▄▄▄  ▄  ▄  ▄ 
 ▄▄▄   ▄▄ ▄▄ ▄▄ ▄ ▄  
 ▄▄ ▄▄▄▄▄▄▄▄ 
 
 
 
 
 
 
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
 
 
 
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
25 changes: 20 additions & 5 deletions all-is-cubes/src/space/light/data.rs
Original file line number Diff line number Diff line change
@@ -45,9 +45,8 @@ pub(crate) enum LightStatus {
Visible = 255,
}

/// Lighting within a [`Space`]; an [`Rgb`] value stored with reduced precision and range.
///
/// TODO: This now stores additional information. Rename to '`SpaceLight`' or some such.
/// A single cube of light within a [`Space`]; an [`Rgb`] value stored with reduced precision and
/// range, plus metadata about the applicability of the result.
#[derive(Clone, Copy, PartialEq, Eq)]
pub struct PackedLight {
// LightStatus being other than Visible is mutually exclusive with value being nonzero,
@@ -202,8 +201,9 @@ impl PackedLight {

#[inline(always)]
fn scalar_in(value: PositiveSign<f32>) -> PackedLightScalar {
// Note that `as` is a saturating cast.
(value.into_inner().log2() * Self::LOG_SCALE + Self::LOG_OFFSET) as PackedLightScalar
// Note that `as` is a saturating cast, so out of range values will be clamped.
(value.into_inner().log2() * Self::LOG_SCALE + Self::LOG_OFFSET).round()
as PackedLightScalar
}

/// Convert a `PackedLightScalar` value to a linear color component value.
@@ -438,6 +438,21 @@ mod tests {
);
}

#[test]
fn packed_light_rounds_to_nearest() {
for i in PackedLightScalar::MIN..PackedLightScalar::MAX {
let value = PackedLight::scalar_out_ps(i);
assert_eq!(
(
PackedLight::scalar_in(value * ps32(0.9999)),
i,
PackedLight::scalar_in(value * ps32(1.0001)),
),
(i, i, i)
);
}
}

#[test]
fn packed_light_is_packed() {
// Technically this is not guaranteed by the compiler, but if it's false something probably went wrong.
15 changes: 8 additions & 7 deletions all-is-cubes/src/space/light/tests.rs
Original file line number Diff line number Diff line change
@@ -191,16 +191,17 @@ fn light_source_self_illumination_opaque() {
assert_eq!(
adjacents,
// TODO: make this test less fragile. The asymmetry isn't even wanted;
// I think it's probably due to exactly diagonal rays.
// I think it's probably due to exactly diagonal rays having to make a choice
// of which neighbors to pass through.
// Some of the values also differ due to our current choice of discarding
// light updates with priority 1.
FaceMap {
nx: Rgb::new(0.13053422, 0.26106843, 0.52213687),
ny: Rgb::new(0.16210495, 0.3242099, 0.6484198),
nz: Rgb::new(0.20131129, 0.40262258, 0.80524516),
px: Rgb::new(0.13053422, 0.26106843, 0.52213687),
py: Rgb::new(0.16210495, 0.3242099, 0.6484198),
pz: Rgb::new(0.20131129, 0.40262258, 0.80524516),
nx: Rgb::new(0.13631347, 0.27262694, 0.5452539),
ny: Rgb::new(0.16928194, 0.3385639, 0.6771278),
nz: Rgb::new(0.2102241, 0.4204482, 0.8408964),
px: Rgb::new(0.13631347, 0.27262694, 0.5452539),
py: Rgb::new(0.16928194, 0.3385639, 0.6771278),
pz: Rgb::new(0.2102241, 0.4204482, 0.8408964),
},
);
}
Binary file modified test-renderers/expected/renderers/bloom-0.0-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/bloom-0.25-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/fog-Abrupt-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/fog-Compromise-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/fog-None-ray.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/fog-None-wgpu.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/fog-Physical-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/icons-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/light-Flat-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/light-None-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/light-Smooth-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/tone_mapping-Clamp-0.5-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test-renderers/expected/renderers/tone_mapping-Clamp-2.0-all.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 9 additions & 2 deletions test-renderers/src/test_cases.rs
Original file line number Diff line number Diff line change
@@ -937,10 +937,17 @@ async fn tone_mapping(mut context: RenderTestContext, (tmo, exposure): (ToneMapp
context.universe(),
);

// TODO: Ideally there would be at most 1 difference.
// TODO: Ideally there would be at most 1 difference but there are large-enough-to-be-visible
// ones for some reason. These appeared, not when working on the tone mapping algorithm, but
// when changing PackedLight’s rounding behavior.
// Also, there is a notable ray/wgpu difference concentrated in the middle range of Reinhard.
// So, there are probably bugs lurking here.
context
.render_comparison_test(Threshold::new([(3, 500)]), scene, Overlays::NONE)
.render_comparison_test(
Threshold::new([(20, 200), (10, 500), (3, 2000), (2, 20000)]),
scene,
Overlays::NONE,
)
.await;
}

0 comments on commit 1e68913

Please sign in to comment.