Update to mapbox/earcut v3.0.1 #19
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
- Coverage 99.56% 99.36% -0.20%
==========================================
Files 2 2
Lines 927 952 +25
==========================================
+ Hits 923 946 +23
- Misses 4 6 +2 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib.rs (2)
290-311: Consider handling infinite or NaN slopes explicitly.When sorting by slope, division by zero or floating overflow might occur. You can guard against infinite or NaN slopes for more robust hole ordering:
let a_slope = (node!(self.nodes, a.next_i).xy[1] - a.xy[1]) - / (node!(self.nodes, a.next_i).xy[0] - a.xy[0]); let b_slope = (node!(self.nodes, b.next_i).xy[1] - b.xy[1]) - / (node!(self.nodes, b.next_i).xy[0] - b.xy[0]); +// Example approach to avoid infinite slope +let dx_a = node!(self.nodes, a.next_i).xy[0] - a.xy[0]; +let a_slope = if dx_a == T::zero() { + T::infinity() +} else { + (node!(self.nodes, a.next_i).xy[1] - a.xy[1]) / dx_a +}; +// similarly for b_slope
1165-1169: Ensure naming reflects partial boundary exclusion.
point_in_triangle_except_firstonly excludes if the point matches the first vertex exactly. If you intend to exclude all boundary points, rename the function or expand the logic to exclude the second and third vertices as well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.tomlis excluded by!**/*.tomltests/fixtures/touching-holes2.jsonis excluded by!**/*.jsontests/fixtures/touching-holes3.jsonis excluded by!**/*.jsontests/fixtures/touching-holes4.jsonis excluded by!**/*.jsontests/fixtures/touching-holes5.jsonis excluded by!**/*.jsontests/fixtures/touching-holes6.jsonis excluded by!**/*.json
📒 Files selected for processing (2)
src/lib.rs(8 hunks)tests/fixture.rs(4 hunks)
🔇 Additional comments (17)
src/lib.rs (7)
429-429: Confirm if excluding the first vertex alone is intended.Replacing
point_in_trianglewithpoint_in_triangle_except_firstexcludes points matching the first vertex. Verify if you also need to exclude the second or third vertex to avoid boundary conditions.
488-488: Same remark as line 429.
500-500: Same remark as line 429.
518-518: Same remark as line 429.
536-536: Same remark as line 429.
885-889: Consider a tolerance-based check for vertex equality.If floating precision errors occur, two points might be logically equal but fail the
equalscheck. Consider using a small epsilon-based approach for robust hole bridging.
893-895: Same remark as lines 885-889.tests/fixture.rs (10)
78-78: Output triangle expectation changed.Ensure 5176 is the correct updated count of triangles for "water-huge".
83-83: Deviation threshold changed.Confirm 0.004 is the desired threshold for "water-huge2".
152-155: Good addition of new test scenario.Thanks for adding "touching-holes2"; it broadens coverage.
156-159: Good addition of new test scenario."touching-holes3" test also improves coverage.
161-164: Added test scenario for holes."touching-holes4" scenario is valuable for verifying hole adjacency logic.
166-169: Another beneficial holes test."touching-holes5" ensures the algorithm handles more complex shapes.
171-174: Expanding coverage with holes."touching-holes6" might be a stress test — thanks for including it.
228-228: Reducing expected triangles from 19 to 18.Confirm that "issue111" is now generating one fewer triangle as intended.
233-233: Updated expected triangles from 57 to 58.Ensure the "boxy" fixture is correct for the new triangulation result.
263-263: Reduced expected triangles from 20 to 19.Check that "touching4" fixture properly reflects the updated triangulation logic.
|
Caution Review failedThe pull request is closed. WalkthroughThis update refines the triangulation algorithm and adjusts test expectations. In the library code, the sorting logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant EarChecker as is_ear
participant PitEx as point_in_triangle_except_first
participant Pit as point_in_triangle
Caller->>EarChecker: Invoke is_ear()
EarChecker->>PitEx: Call point_in_triangle_except_first()
PitEx->>Pit: Call point_in_triangle()
Pit-->>PitEx: Return boolean result
PitEx-->>EarChecker: Return boolean result
EarChecker-->>Caller: Return ear check result
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Closes #18