Ralith / hypermine

A hyperbolic voxel game
Apache License 2.0
160 stars 20 forks source link

Default to f32 instead of f64, removing unnecessary casts #360

Closed patowen closed 7 months ago

patowen commented 9 months ago

Purpose

A common issue I have encountered when trying to implement various features was the fact that much of Hypermine's code relies on f32, but certain common functions, especially in dodeca.cs, only returned f64, requiring a lot of redundant casting.

Changes

This PR changes dodeca to return f32 types by default, with alternative methods with _f64 appended to their name to allow them to return f64 instead.

To take advantage of these changes, some functions, such as the ones in traversal were altered to take in an f32 instead of an f64, ensuring that all math is done with f32. As a side note, this highlighted an oversight in ensure_nearby and nearby_nodes, as a depth-search search was used instead of the superior breadth-first search, causing unnecessary numerical instability (which resulted in NaNs after the switch to f32). This PR also fixes that oversight.

This PR still uses f64 for Plane to avoid making #102 worse, and it uses f64 when interacting with na::RealField because otherwise, casting types to na::RealField does not work. (See https://github.com/dimforge/simba/issues/54)

Moving forward

This PR is incremental in nature, as it does not yet fully solve the f32 vs f64 disconnect. To do that, I can imagine two approaches, depending on what we want Hypermine's engine to be capable of, although there are almost certainly other ways:

Option 1 (Move entirely to f32): Stop accommodating f64 entirely and use f32 for everything, simplifying the code base. Part of this would involve editing math.rs to use f32 instead of na::RealField + Copy, which could simplify things greatly. The main downside would be that some ad-hoc implementations would run into numerical issues more quickly. For instance, the issue related to #102 would be worsened until we fix it.

Option 2 (Embrace f64 as an first-class option): Use traits more consistently to allow our core functions in dodeca and math to work with f32 or f64. It would likely be a good idea for us to define our own trait implemented by f32 and f64 and stop using na::RealField (at least for things that depend on dodeca), since that would give us more control. For instance, it would allow us to write more generic dodeca methods that return f32 or f64 depending on context.