Skip to content

Commit

Permalink
Convert fov_y and view_distance fields to use PositiveSign.
Browse files Browse the repository at this point in the history
This doesn’t buy us a lot because both are already clamped to be
positive when used, but it’s better defined.
  • Loading branch information
kpreid committed Oct 26, 2024
1 parent 47a9186 commit 30ceaed
Show file tree
Hide file tree
Showing 13 changed files with 67 additions and 57 deletions.
14 changes: 14 additions & 0 deletions all-is-cubes-base/src/math/restricted_number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,13 +685,27 @@ pub const fn ps32(value: f32) -> PositiveSign<f32> {
PositiveSign::<f32>::new_strict(value)
}

/// Convenient alias for [`PositiveSign::<f64>::new_strict()`],
/// to be used in tests and pseudo-literals.
#[inline]
pub const fn ps64(value: f64) -> PositiveSign<f64> {
PositiveSign::<f64>::new_strict(value)
}

/// Convenient alias for [`ZeroOne::<f32>::new_strict()`],
/// to be used in tests and pseudo-literals.
#[inline]
pub const fn zo32(value: f32) -> ZeroOne<f32> {
ZeroOne::<f32>::new_strict(value)
}

/// Convenient alias for [`ZeroOne::<f64>::new_strict()`],
/// to be used in tests and pseudo-literals.
#[inline]
pub const fn zo64(value: f64) -> ZeroOne<f64> {
ZeroOne::<f64>::new_strict(value)
}

// -------------------------------------------------------------------------------------------------

#[cfg(test)]
Expand Down
2 changes: 1 addition & 1 deletion all-is-cubes-gpu/src/in_wgpu/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub(crate) struct ShaderSpaceCamera {
impl ShaderSpaceCamera {
pub fn new(camera: &Camera) -> Self {
let options = camera.options();
let view_distance = camera.view_distance() as f32;
let view_distance = camera.view_distance().into_inner() as f32;
let (fog_mode_blend, fog_distance) = match options.fog {
FogOption::Abrupt => (1.0, view_distance),
FogOption::Compromise => (0.5, view_distance),
Expand Down
17 changes: 11 additions & 6 deletions all-is-cubes-gpu/src/in_wgpu/light_texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@ use rayon::{
slice::ParallelSliceMut as _,
};

use all_is_cubes::euclid::{Box3D, Vector3D};
use all_is_cubes::math::{
Aab, Axis, Cube, FaceMap, FreeCoordinate, GridAab, GridCoordinate, GridSize, GridSizeCoord,
};
use all_is_cubes::space::Space;
use all_is_cubes::{
euclid::{Box3D, Vector3D},
math::PositiveSign,
};
use all_is_cubes_render::camera::Camera;

use crate::{
Expand All @@ -36,7 +39,8 @@ const CAMERA_MARGIN_RADIUS: f64 = crate::in_wgpu::space::CHUNK_SIZE as f64 * 1.7
/// This region is bounded by view distance and by `Space` bounds.
fn visible_light_volume(space_bounds: GridAab, camera: &Camera) -> GridAab {
// TODO: handle NaN and overflow cases, and the texture not being big enough, for robustness.
let effective_view_radius = Vector3D::splat(camera.view_distance() + CAMERA_MARGIN_RADIUS);
let effective_view_radius =
Vector3D::splat(camera.view_distance().into_inner() + CAMERA_MARGIN_RADIUS);
let visible_bounds = Aab::from_lower_upper(
camera.view_position() - effective_view_radius,
camera.view_position() + effective_view_radius,
Expand Down Expand Up @@ -83,7 +87,7 @@ impl LightTexture {
pub fn choose_size(
limits: &wgpu::Limits,
space_bounds: GridAab,
view_distance: FreeCoordinate,
view_distance: PositiveSign<FreeCoordinate>,
) -> GridSize {
// Extra volume of 1 extra cube around all sides automatically captures sky light.
let space_size = space_bounds.size() + GridSize::splat(2);
Expand All @@ -92,6 +96,7 @@ impl LightTexture {
// containing cubes.
let camera_size = GridSize::splat(
view_distance
.into_inner()
.mul_add(2., CAMERA_MARGIN_RADIUS.mul_add(2., 1.))
.ceil() as GridSizeCoord,
);
Expand Down Expand Up @@ -445,7 +450,7 @@ fn split_axis(
#[cfg(test)]
mod tests {
use all_is_cubes::euclid::vec3;
use all_is_cubes::math::NotNan;
use all_is_cubes::math::ps64;
use all_is_cubes_render::camera::{GraphicsOptions, ViewTransform, Viewport};

use super::*;
Expand All @@ -465,12 +470,12 @@ mod tests {
// for this.
let step = 1. / 8.;
// note: view distance is clamped in graphics options to be a minimum of 1.0
for view_distance in (8..100).map(|i| f64::from(i) * step) {
for view_distance in (8..100).map(|i| ps64(f64::from(i) * step)) {
let texture_size =
LightTexture::choose_size(&limits, irrelevant_space_bounds, view_distance);

let mut options = GraphicsOptions::default();
options.view_distance = NotNan::new(view_distance).unwrap();
options.view_distance = view_distance;
camera.set_options(options);

for position in (0..100).map(|i| f64::from(i) * step) {
Expand Down
4 changes: 2 additions & 2 deletions all-is-cubes-gpu/src/in_wgpu/rerun_image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ fn perform_image_copy(
let (pinhole, transform) = rg::convert_camera_to_pinhole(camera);

let depth_range = Some(rg::components::ValueRange::new(
camera.near_plane_distance(),
camera.view_distance(),
camera.near_plane_distance().into_inner(),
camera.view_distance().into_inner(),
));

Box::pin(async move {
Expand Down
4 changes: 2 additions & 2 deletions all-is-cubes-gpu/src/in_wgpu/shader_testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use wgpu::util::DeviceExt as _;

use all_is_cubes::euclid::{point3, Rotation3D};
use all_is_cubes::listen::ListenableSource;
use all_is_cubes::math::{notnan, Face6, FreeVector, GridSize, GridVector, Rgba};
use all_is_cubes::math::{ps64, Face6, FreeVector, GridSize, GridVector, Rgba};
use all_is_cubes::time;
use all_is_cubes_mesh::{BlockVertex, Coloring};
use all_is_cubes_render::camera::{Camera, GraphicsOptions, ViewTransform, Viewport};
Expand Down Expand Up @@ -174,7 +174,7 @@ where
});

let mut options = GraphicsOptions::default();
options.fov_y = notnan!(90.0);
options.fov_y = ps64(90.0);
let mut camera = Camera::new(options, output_viewport);
camera.set_view_transform(ViewTransform {
rotation: Rotation3D::identity(),
Expand Down
5 changes: 3 additions & 2 deletions all-is-cubes-mesh/src/dynamic/chunked_mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ where
self.complete_time = None;
}

self.chunk_chart.resize_if_needed(camera.view_distance());
self.chunk_chart
.resize_if_needed(camera.view_distance().into_inner());

let prep_to_update_meshes_time = M::Instant::now();

Expand Down Expand Up @@ -334,7 +335,7 @@ where
// Not urgently needed, though.
let cache_distance = FreeCoordinate::from(CHUNK_SIZE);
let retention_distance_squared =
(camera.view_distance().ceil() + cache_distance).powi(2) as i32;
(camera.view_distance().into_inner().ceil() + cache_distance).powi(2) as i32;
self.chunks.retain(|pos, _| {
pos.min_distance_squared_from(view_chunk) <= retention_distance_squared
});
Expand Down
6 changes: 3 additions & 3 deletions all-is-cubes-mesh/src/dynamic/chunked_mesh/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use all_is_cubes::chunking::ChunkPos;
use all_is_cubes::color_block;
use all_is_cubes::content::make_some_blocks;
use all_is_cubes::listen::Listener as _;
use all_is_cubes::math::{zo32, GridPoint, NotNan};
use all_is_cubes::math::{zo32, GridPoint};
use all_is_cubes::math::{Cube, FreePoint, GridAab, GridCoordinate};
use all_is_cubes::space::{BlockIndex, Space, SpaceChange, SpaceTransaction};
use all_is_cubes::time;
Expand Down Expand Up @@ -167,7 +167,7 @@ impl<const MBM: usize> CsmTester<MBM> {
let camera = Camera::new(
{
let mut o = GraphicsOptions::default();
o.view_distance = NotNan::new(view_distance).unwrap();
o.view_distance = view_distance.try_into().unwrap();
o
},
Viewport::ARBITRARY,
Expand Down Expand Up @@ -268,7 +268,7 @@ fn sort_view_every_frame_only_if_transparent() {
fn graphics_options_change() {
// TODO: This test is fragile because it doesn't think about multiple chunks.
let mut options = GraphicsOptions::default();
options.view_distance = NotNan::from(1);
options.view_distance = 1u8.into();
options.transparency = TransparencyOption::Volumetric;

let mut space = Space::empty_positive(1, 1, 1);
Expand Down
2 changes: 1 addition & 1 deletion all-is-cubes-port/src/gltf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ fn convert_camera(name: Option<String>, camera: &Camera) -> gltf_json::Camera {
aspect_ratio: Some(camera.viewport().nominal_aspect_ratio() as f32),
yfov: camera.options().fov_y.into_inner() as f32 * (std::f32::consts::PI / 180.),
zfar: Some(camera.options().view_distance.into_inner() as f32),
znear: camera.near_plane_distance() as f32,
znear: camera.near_plane_distance().into_inner() as f32,
extensions: Default::default(),
extras: Default::default(),
}),
Expand Down
5 changes: 2 additions & 3 deletions all-is-cubes-ui/src/ui_content/vui_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use all_is_cubes::arcstr::ArcStr;
use all_is_cubes::character::{Character, Cursor};
use all_is_cubes::inv::{EphemeralOpaque, Tool, ToolError, ToolInput};
use all_is_cubes::listen::{DirtyFlag, ListenableCell, ListenableSource, Notifier};
use all_is_cubes::math::NotNan;
use all_is_cubes::space::Space;
use all_is_cubes::time;
use all_is_cubes::transaction::{self, Transaction};
Expand Down Expand Up @@ -277,14 +276,14 @@ impl Vui {
/// Compute graphics options to render the VUI space given the user's regular options.
fn graphics_options(mut options: GraphicsOptions) -> GraphicsOptions {
// Set FOV to give a predictable, not-too-wide-angle perspective.
options.fov_y = NotNan::from(30);
options.fov_y = 30u8.into();

// Disable fog for maximum clarity and because we shouldn't have any far clipping to hide.
options.fog = FogOption::None;

// Fixed view distance for our layout.
// TODO: Derive this from HudLayout and also FOV (since FOV determines eye-to-space distance).
options.view_distance = NotNan::from(100);
options.view_distance = 100u8.into();

// clutter
options.debug_chunk_boxes = false;
Expand Down
13 changes: 7 additions & 6 deletions all-is-cubes/src/camera.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Note: This module is hidden, and its contents re-exported as `all_is_cubes_render::camera`.
use all_is_cubes_base::math::ps64;
use euclid::{
point3, vec3, Angle, Point2D, Point3D, RigidTransform3D, Rotation3D, Size2D, Transform3D,
};
Expand Down Expand Up @@ -189,15 +190,15 @@ impl Camera {

/// Returns the view distance; the far plane of the view frustum, or the distance
/// at which rendering may be truncated.
pub fn view_distance(&self) -> FreeCoordinate {
self.options.view_distance.into_inner()
pub fn view_distance(&self) -> PositiveSign<FreeCoordinate> {
self.options.view_distance
}

/// Returns the position of the near plane of the view frustum.
/// This is not currently configurable.
pub fn near_plane_distance(&self) -> FreeCoordinate {
pub fn near_plane_distance(&self) -> PositiveSign<FreeCoordinate> {
// half a voxel at resolution=16
(32.0f64).recip()
ps64((32.0f64).recip())
}

/// Returns a perspective projection matrix based on the configured FOV and view distance,
Expand Down Expand Up @@ -378,8 +379,8 @@ impl Camera {
let fov_cot = (self.fov_y() / 2.).to_radians().tan().recip();
let aspect = self.viewport.nominal_aspect_ratio();

let near = self.near_plane_distance();
let far = self.view_distance();
let near = self.near_plane_distance().into_inner();
let far = self.view_distance().into_inner();

// Rationale for this particular matrix formula: "that's what `cgmath` does",
// and we used to use `cgmath`.
Expand Down
28 changes: 9 additions & 19 deletions all-is-cubes/src/camera/graphics_options.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use core::fmt;

use num_traits::ConstOne as _;
use ordered_float::NotNan;

use crate::math::{notnan, zo32, FreeCoordinate, PositiveSign, Rgb, Rgba, ZeroOne};
use crate::math::{ps64, zo32, FreeCoordinate, PositiveSign, Rgb, Rgba, ZeroOne};
use crate::util::ShowStatus;

#[cfg(doc)]
Expand All @@ -18,13 +17,6 @@ use crate::{block::Block, space::Space};
// (Due to crate splitting that can't be a doc-link.)
#[doc = include_str!("../save/serde-warning.md")]
#[derive(Clone, Eq, PartialEq)]
#[cfg_attr(
feature = "save",
expect(
clippy::unsafe_derive_deserialize,
reason = "false positive from notnan! macro"
)
)]
#[cfg_attr(feature = "save", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "save", serde(default))]
#[non_exhaustive]
Expand All @@ -41,7 +33,7 @@ pub struct GraphicsOptions {
pub fog: FogOption,

/// Field of view, in degrees from top to bottom edge of the viewport.
pub fov_y: NotNan<FreeCoordinate>,
pub fov_y: PositiveSign<FreeCoordinate>,

/// Method to use to remap colors to fit within the displayable range.
pub tone_mapping: ToneMappingOperator,
Expand All @@ -58,7 +50,7 @@ pub struct GraphicsOptions {
/// Distance, in unit cubes, from the camera to the farthest visible point.
///
/// TODO: Implement view distance limit (and fog) in raytracer.
pub view_distance: NotNan<FreeCoordinate>,
pub view_distance: PositiveSign<FreeCoordinate>,

/// Style in which to draw the lighting of [`Space`](crate::space::Space)s.
/// This does not affect the *computation* of lighting.
Expand Down Expand Up @@ -130,12 +122,12 @@ impl GraphicsOptions {
pub const UNALTERED_COLORS: Self = Self {
render_method: RenderMethod::Preferred,
fog: FogOption::None,
fov_y: notnan!(90.),
fov_y: ps64(90.),
// TODO: Change tone mapping default once we have a good implementation.
tone_mapping: ToneMappingOperator::Clamp,
exposure: ExposureOption::Fixed(PositiveSign::<f32>::ONE),
bloom_intensity: zo32(0.),
view_distance: notnan!(200.),
view_distance: ps64(200.),
lighting_display: LightingOption::None,
transparency: TransparencyOption::Volumetric,
show_ui: true,
Expand All @@ -152,10 +144,8 @@ impl GraphicsOptions {
/// Constrain fields to valid/practical values.
#[must_use]
pub fn repair(mut self) -> Self {
self.fov_y = self.fov_y.clamp(NotNan::from(1), NotNan::from(189));
self.view_distance = self
.view_distance
.clamp(NotNan::from(1), NotNan::from(10000));
self.fov_y = self.fov_y.clamp(ps64(1.), ps64(189.));
self.view_distance = self.view_distance.clamp(ps64(1.), ps64(10000.));
self
}
}
Expand Down Expand Up @@ -215,12 +205,12 @@ impl Default for GraphicsOptions {
Self {
render_method: RenderMethod::Preferred,
fog: FogOption::Abrupt,
fov_y: NotNan::from(90),
fov_y: ps64(90.),
// TODO: Change tone mapping default once we have a good implementation.
tone_mapping: ToneMappingOperator::Clamp,
exposure: ExposureOption::default(),
bloom_intensity: zo32(0.125),
view_distance: NotNan::from(200),
view_distance: ps64(200.),
lighting_display: LightingOption::Smooth,
transparency: TransparencyOption::Volumetric,
show_ui: true,
Expand Down
10 changes: 5 additions & 5 deletions all-is-cubes/src/camera/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::camera::{
look_at_y_up, Camera, ExposureOption, FrustumPoints, GraphicsOptions, LightingOption,
ViewTransform, Viewport,
};
use crate::math::{ps32, rgba_const, Aab, NotNan};
use crate::math::{ps32, ps64, rgba_const, Aab};

#[test]
fn camera_bad_viewport_doesnt_panic() {
Expand All @@ -25,7 +25,7 @@ fn set_options_updates_matrices() {
let matrix = camera.projection_matrix();
camera.set_options({
let mut g = camera.options().clone();
g.fov_y = NotNan::from(30);
g.fov_y = ps64(30.);
g
});
assert_ne!(matrix, camera.projection_matrix());
Expand Down Expand Up @@ -54,7 +54,7 @@ fn projection_depth() {
let world_depths = [camera.near_plane_distance(), camera.view_distance()];
let expected_ndc_depths = [0., 1.];
let actual_ndc_depths = world_depths.map(|z| {
let eye = point3(0., 0., -z);
let eye = point3(0., 0., -f64::from(z));
let clip = mat.transform_point3d_homogeneous(eye);
// doesn't reject z=0 like euclid's to_point3d() does
clip.z / clip.w
Expand All @@ -71,8 +71,8 @@ fn projection_depth() {
fn view_frustum() {
let camera = Camera::new(
GraphicsOptions {
view_distance: NotNan::from(10i32.pow(2)),
fov_y: NotNan::from(90),
view_distance: ps64(10f64.powi(2)),
fov_y: ps64(90.),
..GraphicsOptions::default()
},
Viewport::with_scale(1.0, [10, 5]),
Expand Down
Loading

0 comments on commit 30ceaed

Please sign in to comment.