From c36a40645afe4327facc376daeb441065bdca058 Mon Sep 17 00:00:00 2001 From: Graham Pentheny Date: Mon, 1 Jan 2024 14:56:47 -0500 Subject: [PATCH 24/36] Cleanup filter code and improved documentation (#683) This mostly just changes variable names and adds some comments to make the code more clear. It also has a few small fixup changes to the unit tests. --- Recast/Include/Recast.h | 3 +- Recast/Source/RecastFilter.cpp | 126 +++++++++++++++------------- Tests/Recast/Tests_RecastFilter.cpp | 56 +------------ 3 files changed, 75 insertions(+), 110 deletions(-) diff --git a/Recast/Include/Recast.h b/Recast/Include/Recast.h index 2104aa5..5b34ea5 100644 --- a/Recast/Include/Recast.h +++ b/Recast/Include/Recast.h @@ -1002,7 +1002,8 @@ bool rcRasterizeTriangles(rcContext* context, /// Marks non-walkable spans as walkable if their maximum is within @p walkableClimb of the span below them. /// -/// This removes small obstacles that the agent would be able to walk over such as curbs, and also allows agents to move up structures such as stairs. +/// This removes small obstacles and rasterization artifacts that the agent would be able to walk over +/// such as curbs. It also allows agents to move up terraced structures like stairs. /// /// Obstacle spans are marked walkable if: obstacleSpan.smax - walkableSpan.smax < walkableClimb /// diff --git a/Recast/Source/RecastFilter.cpp b/Recast/Source/RecastFilter.cpp index 1ecf858..7875040 100644 --- a/Recast/Source/RecastFilter.cpp +++ b/Recast/Source/RecastFilter.cpp @@ -41,24 +41,24 @@ void rcFilterLowHangingWalkableObstacles(rcContext* context, const int walkableC { rcSpan* previousSpan = NULL; bool previousWasWalkable = false; - unsigned char previousArea = RC_NULL_AREA; + unsigned char previousAreaID = RC_NULL_AREA; + // For each span in the column... for (rcSpan* span = heightfield.spans[x + z * xSize]; span != NULL; previousSpan = span, span = span->next) { const bool walkable = span->area != RC_NULL_AREA; - // If current span is not walkable, but there is walkable - // span just below it, mark the span above it walkable too. - if (!walkable && previousWasWalkable) + + // If current span is not walkable, but there is walkable span just below it and the height difference + // is small enough for the agent to walk over, mark the current span as walkable too. + if (!walkable && previousWasWalkable && (int)span->smax - (int)previousSpan->smax <= walkableClimb) { - if (rcAbs((int)span->smax - (int)previousSpan->smax) <= walkableClimb) - { - span->area = previousArea; - } + span->area = previousAreaID; } - // Copy walkable flag so that it cannot propagate - // past multiple non-walkable objects. + + // Copy the original walkable value regardless of whether we changed it. + // This prevents multiple consecutive non-walkable spans from being erroneously marked as walkable. previousWasWalkable = walkable; - previousArea = span->area; + previousAreaID = span->area; } } } @@ -73,84 +73,98 @@ void rcFilterLedgeSpans(rcContext* context, const int walkableHeight, const int const int xSize = heightfield.width; const int zSize = heightfield.height; - // Mark border spans. + // Mark spans that are adjacent to a ledge as unwalkable.. for (int z = 0; z < zSize; ++z) { for (int x = 0; x < xSize; ++x) { for (rcSpan* span = heightfield.spans[x + z * xSize]; span; span = span->next) { - // Skip non walkable spans. + // Skip non-walkable spans. if (span->area == RC_NULL_AREA) { continue; } - const int bot = (int)(span->smax); - const int top = span->next ? (int)(span->next->smin) : MAX_HEIGHTFIELD_HEIGHT; + const int floor = (int)(span->smax); + const int ceiling = span->next ? (int)(span->next->smin) : MAX_HEIGHTFIELD_HEIGHT; - // Find neighbours minimum height. - int minNeighborHeight = MAX_HEIGHTFIELD_HEIGHT; + // The difference between this walkable area and the lowest neighbor walkable area. + // This is the difference between the current span and all neighbor spans that have + // enough space for an agent to move between, but not accounting at all for surface slope. + int lowestNeighborFloorDifference = MAX_HEIGHTFIELD_HEIGHT; // Min and max height of accessible neighbours. - int accessibleNeighborMinHeight = span->smax; - int accessibleNeighborMaxHeight = span->smax; + int lowestTraversableNeighborFloor = span->smax; + int highestTraversableNeighborFloor = span->smax; for (int direction = 0; direction < 4; ++direction) { - int dx = x + rcGetDirOffsetX(direction); - int dz = z + rcGetDirOffsetY(direction); + const int neighborX = x + rcGetDirOffsetX(direction); + const int neighborZ = z + rcGetDirOffsetY(direction); + // Skip neighbours which are out of bounds. - if (dx < 0 || dz < 0 || dx >= xSize || dz >= zSize) + if (neighborX < 0 || neighborZ < 0 || neighborX >= xSize || neighborZ >= zSize) { - minNeighborHeight = (-walkableClimb - 1) ; + lowestNeighborFloorDifference = -walkableClimb - 1; break; } - // From minus infinity to the first span. - const rcSpan* neighborSpan = heightfield.spans[dx + dz * xSize]; - int neighborTop = neighborSpan ? (int)neighborSpan->smin : MAX_HEIGHTFIELD_HEIGHT; - + const rcSpan* neighborSpan = heightfield.spans[neighborX + neighborZ * xSize]; + + // The most we can step down to the neighbor is the walkableClimb distance. + // Start with the area under the neighbor span + int neighborCeiling = neighborSpan ? (int)neighborSpan->smin : MAX_HEIGHTFIELD_HEIGHT; + // Skip neighbour if the gap between the spans is too small. - if (rcMin(top, neighborTop) - bot >= walkableHeight) + if (rcMin(ceiling, neighborCeiling) - floor >= walkableHeight) { - minNeighborHeight = (-walkableClimb - 1); + lowestNeighborFloorDifference = (-walkableClimb - 1); break; } - // Rest of the spans. - for (neighborSpan = heightfield.spans[dx + dz * xSize]; neighborSpan; neighborSpan = neighborSpan->next) + // For each span in the neighboring column... + for (; neighborSpan != NULL; neighborSpan = neighborSpan->next) { - int neighborBot = (int)neighborSpan->smax; - neighborTop = neighborSpan->next ? (int)neighborSpan->next->smin : MAX_HEIGHTFIELD_HEIGHT; - - // Skip neighbour if the gap between the spans is too small. - if (rcMin(top, neighborTop) - rcMax(bot, neighborBot) >= walkableHeight) + const int neighborFloor = (int)neighborSpan->smax; + neighborCeiling = neighborSpan->next ? (int)neighborSpan->next->smin : MAX_HEIGHTFIELD_HEIGHT; + + // Only consider neighboring areas that have enough overlap to be potentially traversable. + if (rcMin(ceiling, neighborCeiling) - rcMax(floor, neighborFloor) < walkableHeight) + { + // No space to traverse between them. + continue; + } + + const int neighborFloorDifference = neighborFloor - floor; + lowestNeighborFloorDifference = rcMin(lowestNeighborFloorDifference, neighborFloorDifference); + + // Find min/max accessible neighbor height. + // Only consider neighbors that are at most walkableClimb away. + if (rcAbs(neighborFloorDifference) <= walkableClimb) + { + // There is space to move to the neighbor cell and the slope isn't too much. + lowestTraversableNeighborFloor = rcMin(lowestTraversableNeighborFloor, neighborFloor); + highestTraversableNeighborFloor = rcMax(highestTraversableNeighborFloor, neighborFloor); + } + else if (neighborFloorDifference < -walkableClimb) { - int accessibleNeighbourHeight = neighborBot - bot; - minNeighborHeight = rcMin(minNeighborHeight, accessibleNeighbourHeight); - - // Find min/max accessible neighbour height. - if (rcAbs(accessibleNeighbourHeight) <= walkableClimb) - { - if (neighborBot < accessibleNeighborMinHeight) accessibleNeighborMinHeight = neighborBot; - if (neighborBot > accessibleNeighborMaxHeight) accessibleNeighborMaxHeight = neighborBot; - } - else if (accessibleNeighbourHeight < -walkableClimb) - { - break; - } + // We already know this will be considered a ledge span so we can early-out + break; } } } - // The current span is close to a ledge if the drop to any neighbour span is less than the walkableClimb. - if (minNeighborHeight < -walkableClimb) + // The current span is close to a ledge if the magnitude of the drop to any neighbour span is greater than the walkableClimb distance. + // That is, there is a gap that is large enough to let an agent move between them, but the drop (surface slope) is too large to allow it. + // (If this is the case, then biggestNeighborStepDown will be negative, so compare against the negative walkableClimb as a means of checking + // the magnitude of the delta) + if (lowestNeighborFloorDifference < -walkableClimb) { span->area = RC_NULL_AREA; } - // If the difference between all neighbours is too large, we are at steep slope, mark the span as ledge. - else if ((accessibleNeighborMaxHeight - accessibleNeighborMinHeight) > walkableClimb) + // If the difference between all neighbor floors is too large, this is a steep slope, so mark the span as an unwalkable ledge. + else if (highestTraversableNeighborFloor - lowestTraversableNeighborFloor > walkableClimb) { span->area = RC_NULL_AREA; } @@ -175,9 +189,9 @@ void rcFilterWalkableLowHeightSpans(rcContext* context, const int walkableHeight { for (rcSpan* span = heightfield.spans[x + z*xSize]; span; span = span->next) { - const int bot = (int)(span->smax); - const int top = span->next ? (int)(span->next->smin) : MAX_HEIGHTFIELD_HEIGHT; - if ((top - bot) < walkableHeight) + const int floor = (int)(span->smax); + const int ceiling = span->next ? (int)(span->next->smin) : MAX_HEIGHTFIELD_HEIGHT; + if (ceiling - floor < walkableHeight) { span->area = RC_NULL_AREA; } diff --git a/Tests/Recast/Tests_RecastFilter.cpp b/Tests/Recast/Tests_RecastFilter.cpp index 1882adb..d600835 100644 --- a/Tests/Recast/Tests_RecastFilter.cpp +++ b/Tests/Recast/Tests_RecastFilter.cpp @@ -47,7 +47,7 @@ TEST_CASE("rcFilterLowHangingWalkableObstacles", "[recast, filtering]") { // Put the second span just above the first one. rcSpan* secondSpan = (rcSpan*)rcAlloc(sizeof(rcSpan), RC_ALLOC_PERM); - secondSpan->area = 1; + secondSpan->area = RC_NULL_AREA; secondSpan->next = NULL; secondSpan->smin = 1 + walkableHeight; secondSpan->smax = secondSpan->smin + 1; @@ -64,7 +64,7 @@ TEST_CASE("rcFilterLowHangingWalkableObstacles", "[recast, filtering]") // Check that nothing has changed. REQUIRE(heightfield.spans[0]->area == 1); - REQUIRE(heightfield.spans[0]->next->area == 1); + REQUIRE(heightfield.spans[0]->next->area == RC_NULL_AREA); // Check again but with a more clearance secondSpan->smin += 10; @@ -74,7 +74,7 @@ TEST_CASE("rcFilterLowHangingWalkableObstacles", "[recast, filtering]") // Check that nothing has changed. REQUIRE(heightfield.spans[0]->area == 1); - REQUIRE(heightfield.spans[0]->next->area == 1); + REQUIRE(heightfield.spans[0]->next->area == RC_NULL_AREA); rcFree(span); rcFree(secondSpan); @@ -251,56 +251,6 @@ TEST_CASE("rcFilterLedgeSpans", "[recast, filtering]") } } } - - SECTION("Edge spans are marked unwalkable") - { - // Create a flat plane. - for (int x = 0; x < heightfield.width; ++x) - { - for (int z = 0; z < heightfield.height; ++z) - { - rcSpan* span = (rcSpan*)rcAlloc(sizeof(rcSpan), RC_ALLOC_PERM); - span->area = 1; - span->next = NULL; - span->smin = 0; - span->smax = 1; - heightfield.spans[x + z * heightfield.width] = span; - } - } - - rcFilterLedgeSpans(&context, walkableHeight, walkableClimb, heightfield); - - for (int x = 0; x < heightfield.width; ++x) - { - for (int z = 0; z < heightfield.height; ++z) - { - rcSpan* span = heightfield.spans[x + z * heightfield.width]; - REQUIRE(span != NULL); - - if (x == 0 || z == 0 || x == 9 || z == 9) - { - REQUIRE(span->area == RC_NULL_AREA); - } - else - { - REQUIRE(span->area == 1); - } - - REQUIRE(span->next == NULL); - REQUIRE(span->smin == 0); - REQUIRE(span->smax == 1); - } - } - - // Free all the heightfield spans - for (int x = 0; x < heightfield.width; ++x) - { - for (int z = 0; z < heightfield.height; ++z) - { - rcFree(heightfield.spans[x + z * heightfield.width]); - } - } - } } TEST_CASE("rcFilterWalkableLowHeightSpans", "[recast, filtering]") -- 2.43.0