From 83bf5d099f68818c818b5e153b52725a7ba988ad Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sun, 30 Jun 2019 15:16:14 +0200 Subject: [PATCH] Remove unnecessary floats and simplify code Replace floats with new zlight_slope struct. This now passes 2 of the tests that the old recursive algorithm was failing. This encounters an infinite loop in some cases however without the fuzzing that seems to have been hiding a major bug. --- src/shadowcasting.cpp | 115 ++++++++++++++++++++++++++++-------------- 1 file changed, 76 insertions(+), 39 deletions(-) diff --git a/src/shadowcasting.cpp b/src/shadowcasting.cpp index 62ae932a75cb1..3dbdf4fca8630 100644 --- a/src/shadowcasting.cpp +++ b/src/shadowcasting.cpp @@ -6,18 +6,59 @@ #include "fragment_cloud.h" // IWYU pragma: keep #include "line.h" +struct slope { + slope( int rise, int run ) { + // Ensure run is always positive for the comparison operators + this->run = abs( run ); + if( run < 0 ) { + this->rise = -rise; + } else { + this->rise = rise; + } + } + + int run; + int rise; +}; + +inline bool operator<( const slope &lhs, const slope &rhs ) +{ + // a/b < c/d <=> a*d < c*b if b and d have the same sign. + return lhs.rise * rhs.run < rhs.rise * lhs.run; +} +inline bool operator>( const slope &lhs, const slope &rhs ) +{ + return rhs < lhs; +} +inline bool operator<=( const slope &lhs, const slope &rhs ) +{ + return !( lhs > rhs ); +} +inline bool operator>=( const slope &lhs, const slope &rhs ) +{ + return !( lhs < rhs ); +} +inline bool operator==( const slope &lhs, const slope &rhs ) +{ + // a/b == c/d <=> a*d == c*b if b and d have the same sign. + return lhs.rise * rhs.run == rhs.rise * lhs.run; +} +inline bool operator!=( const slope &lhs, const slope &rhs ) +{ + return !( lhs == rhs ); +} + template struct span { - span( const float &s_major, const float &e_major, - const float &s_minor, const float &e_minor, + span( const slope &s_major, const slope &e_major, + const slope &s_minor, const slope &e_minor, const T &value ) : start_major( s_major ), end_major( e_major ), start_minor( s_minor ), end_minor( e_minor ), cumulative_value( value ) {} - // TODO: Make these fixed-point or byte/byte pairs - float start_major; - float end_major; - float start_minor; - float end_minor; + slope start_major; + slope end_major; + slope start_minor; + slope end_minor; T cumulative_value; }; @@ -44,12 +85,12 @@ void cast_zlight_segment( const tripoint &offset, const int offset_distance, const T numerator ) { - const float radius = 60.0f - offset_distance; + const int radius = 60 - offset_distance; constexpr int min_z = -OVERMAP_DEPTH; constexpr int max_z = OVERMAP_HEIGHT; - float new_start_minor = 1.0f; + slope new_start_minor( 1, 1 ); T last_intensity = 0.0; static constexpr tripoint origin( 0, 0, 0 ); @@ -59,19 +100,24 @@ void cast_zlight_segment( // We start out with one span covering the entire horizontal and vertical space // we are interested in. Then as changes in transparency are encountered, we truncate // that initial span and insert new spans after it in the list. - std::list> spans = { { 0.0, 1.0, 0.0, 1.0, LIGHT_TRANSPARENCY_OPEN_AIR } }; + std::list> spans = { { + slope( 0, 1 ), slope( 1, 1 ), + slope( 0, 1 ), slope( 1, 1 ), + LIGHT_TRANSPARENCY_OPEN_AIR + } + }; // At each "depth", a.k.a. distance from the origin, we iterate once over the list of spans, // possibly splitting them. for( int distance = 1; distance <= radius; distance++ ) { delta.y = distance; bool started_block = false; - T current_transparency = 0.0f; + T current_transparency; for( auto this_span = spans.begin(); this_span != spans.end(); ++this_span ) { // TODO: Precalculate min/max delta.z based on start/end and distance for( delta.z = 0; delta.z <= distance; delta.z++ ) { - const float trailing_edge_major = ( delta.z - 0.5f ) / ( delta.y + 0.5f ); - const float leading_edge_major = ( delta.z + 0.5f ) / ( delta.y - 0.5f ); + const slope trailing_edge_major( delta.z * 2 - 1, delta.y * 2 + 1 ); + const slope leading_edge_major( delta.z * 2 + 1, delta.y * 2 - 1 ); current.z = offset.z + delta.x * 00 + delta.y * 00 + delta.z * zz; if( current.z > max_z || current.z < min_z ) { continue; @@ -90,24 +136,24 @@ void cast_zlight_segment( for( delta.x = 0; delta.x <= distance; delta.x++ ) { current.x = offset.x + delta.x * xx + delta.y * xy + delta.z * xz; current.y = offset.y + delta.x * yx + delta.y * yy + delta.z * yz; - // Shadowcasting sweeps from the most extreme edge of the octant to the cardinal + // Shadowcasting sweeps from the cardinal to the most extreme edge of the octant // XXXX - // <--- + // ---> // XXX - // <-- + // --> // XX - // <- + // -> // X // @ // - // Leading edge -> +- - // |+| <- Center of tile - // -+ <- Trailing edge - // <------ Direction of sweep + // Trailing edge -> +- + // |+| <- Center of tile + // -+ <- Leading Edge + // Direction of sweep ---> // Use corners of given tile as above to determine angles of // leading and trailing edges being considered. - float trailing_edge_minor = ( delta.x - 0.5f ) / ( delta.y + 0.5f ); - float leading_edge_minor = ( delta.x + 0.5f ) / ( delta.y - 0.5f ); + const slope trailing_edge_minor( delta.x * 2 - 1, delta.y * 2 + 1 ); + const slope leading_edge_minor( delta.x * 2 + 1, delta.y * 2 - 1 ); if( !( current.x >= 0 && current.y >= 0 && current.x < MAPSIZE_X && current.y < MAPSIZE_Y ) || this_span->start_minor > leading_edge_minor ) { @@ -151,8 +197,6 @@ void cast_zlight_segment( if( !started_span ) { // Need to reset minor slope, because we're starting a new line new_start_minor = leading_edge_minor; - // Need more precision or artifacts happen - leading_edge_minor = this_span->start_minor; started_span = true; } @@ -168,9 +212,9 @@ void cast_zlight_segment( // this is the view from the origin looking out): // +-------+ <- end major // | D | - // +---+---+ <- end major mid + // +---+---+ <- leading edge major // | B | C | - // +---+---+ <- start major mid + // +---+---+ <- trailing edge major // | A | // +-------+ <- start major // ^ ^ @@ -190,8 +234,6 @@ void cast_zlight_segment( // If this is the last row processed, there is no D span. // If check returns false, A and B are opaque and have no spans. if( check( current_transparency, last_intensity ) ) { - // Save this in case this_span changes - const float old_start_major = this_span->start_major; // Emit the A span if it's present. if( trailing_edge_major > this_span->start_major ) { // Place BCD next in the list @@ -206,9 +248,7 @@ void cast_zlight_segment( } // One way or the other, the current span is now BCD. // First handle B. - const float start_major_mid = std::max( old_start_major, trailing_edge_major ); - const float end_major_mid = std::min( this_span->end_major, leading_edge_major ); - spans.emplace( this_span, start_major_mid, end_major_mid, + spans.emplace( this_span, this_span->start_major, leading_edge_major, this_span->start_minor, trailing_edge_minor, next_cumulative_transparency ); // Overwrite new_start_minor since previous tile is transparent. @@ -217,18 +257,15 @@ void cast_zlight_segment( // Current span is BCD, but B either doesn't exist or is handled. // Split off D if it exists. if( leading_edge_major < this_span->end_major ) { + // Infinite loop where the same D gets split off every iteration currently spans.emplace( std::next( this_span ), - // Fuzz it a little to avoid an infinite loop in some cases - // Something similar was done in the old recursive algorithm - // as well. - // Probably indicative of a bug that needs fixing - leading_edge_major + 0.00001f, this_span->end_major, + leading_edge_major, this_span->end_major, this_span->start_minor, this_span->end_minor, this_span->cumulative_value ); } // Truncate this_span to the current block. - this_span->start_major = std::max( this_span->start_major, trailing_edge_major ); - this_span->end_major = std::min( this_span->end_major, leading_edge_major ); + this_span->start_major = trailing_edge_major; + this_span->end_major = leading_edge_major; // The new span starts at the leading edge of the previous square if it is opaque, // and at the trailing edge of the current square if it is transparent. this_span->start_minor = new_start_minor;