fix(overlay): boolean-operation and buffer correctness fixes - #37
Merged
Conversation
`trace_rings` closed a ring only when the walk returned to the node it started from. Where two lobes of the result touch at one point, the traversal passes through that point twice, so carrying on to the seed spliced the lobes into a single self-touching ring — geometry that `is_valid` rejects, and that `boost::geometry::intersection` never produces. Close the ring at any node already on the current walk instead, emit it, and carry on from there. This is the rule Boost's own traversal follows, and the one the `geometry/correct.hpp` dissolve uses. Verified against C++ Boost 1.83, which returns two valid polygons for the case now covered by `intersection_splits_lobes_meeting_at_a_point`: box (0,0)-(10,10) ring (5,-1) (6,-2) (2,-3) (1,1) (3,2) (2,6) (4,7) (5,0) (6,1) (7,-1) before: one ring through (5,0) twice, area 16.125 but self-touching after: (1.25,0)(1,1)(3,2)(2,6)(4,7)(5,0) and (5,0)(6,1)(6.5,0) Found while scoring this crate against C++ Boost on real polygon/box clips taken from a tilemaker run: it takes those from 3/10 to 6/10 matching Boost's ring set, with no change to the rest of the suite.
`is_valid_ring_with` tested orientation last, so a ring that was both wound the wrong way and self-intersecting came back `SelfIntersection`. Boost reports `failure_wrong_orientation` for it: orientation is checked first, and only a correctly wound ring goes on to be reported as self-intersecting. Spikes still precede both. The distinction is not cosmetic for callers that branch on the code — tilemaker's `buildWayGeometry` redoes its clip on `failure_self_intersections` and leaves the geometry alone on `failure_wrong_orientation`. Measured against Boost 1.83 on 2000 random rings (clean, bow-tie, figure-of-eight and meander shapes, both windings): 203 of the first 400 disagreed before, 0 of 2000 after. The order the new `orientation_is_reported_before_self_intersection` test pins: ccw + real spike failure=12 area=-16 ccw + selfint, no spike failure=22 area=-17 cw self-int, +area failure=21 area=+98 bowtie failure=22 area=+0 Two existing bow-tie assertions expected the old order; a bow-tie's lobes cancel to zero signed area, so Boost fails it on orientation. Updated with the measured value.
The predicate required the dot product to be strictly negative, so a zero-length step never qualified and every repeated vertex survived. Boost's is `point_is_spike_or_equal` — the `or_equal` half is what collapses a duplicate, and `remove_spikes` needs it to clean up after itself: peeling the apex off `(4,0) (6,0) (4,0)` leaves `(4,0) (4,0)` adjacent, which a spike-only predicate then leaves in the output. `dot <= 0` cannot over-match. Two non-zero steps that are both parallel (`cross == 0`) and perpendicular (`dot == 0`) do not exist, so the new arm fires only when one step has zero length. Boost 1.83 on a clockwise `model::polygon`, all five now covered by `repeated_vertices_are_collapsed`: consecutive dup -> (0,0) (0,4) (4,4) (4,0) (0,0) dup at start -> (0,0) (0,4) (4,4) (4,0) (0,0) triple dup -> (0,0) (0,4) (4,4) (4,0) (0,0) real spike -> (0,0) (0,4) (4,4) (4,0) (0,0) dup + spike -> (0,0) (0,4) (4,4) (4,0) (0,0) `is_valid`'s own spike predicate stays strict and keeps `dot < 0`: Boost's default validity policy accepts duplicate points, so the two answer different questions. Noted where they diverge. Measured against Boost on 2000 random rings: 1 in 400 disagreed before, 0 of 2000 after.
`centroid` was a compile error on a multi-polygon; Boost has had one all along. Adds `CartesianMultiPolygonCentroid` and the `MultiPolygonTag` picker impl. It runs one Bashein-Detmer accumulator over every ring of every member and divides once, which is `centroid_multi` in `boost/geometry/algorithms/centroid.hpp` — not a combine of per-member centroids weighted by area. The two are not the same function. A member with zero area has zero weight and vanishes from the weighted combine, but its numerator is not zero and Boost keeps it. `CartesianPolygonCentroid` had the same shape and the same gap, so it now accumulates too. Boost 1.83 on a polygon whose exterior is a bow-tie and whose hole has real area: bowtie exterior + hole -> (10.6667, 11) area=-4 weighted combine gives -> (11, 11) and across members: zero-area MP -> (0, 0) area=0 mixed MP -> (11.3333, 11) area=4 Both are the accumulated answer, and both are now covered by `a_zero_area_part_still_moves_the_centroid`. The existing polygon tests are unchanged: for a polygon with well-formed rings the two formulations agree identically, because the per-ring area cancels against the divisor.
…rong Every inter-member defect was reported as `IntersectingInteriors`. Boost reports `failure_self_intersections` for most of them and reserves `failure_intersecting_interiors` for genuine nesting. Boost 1.83: overlapping members failure=21 boundaries cross edge-touching members failure=21 boundaries share a curve identical members failure=21 one inside another failure=40 interiors overlap, boundaries never meet point-touching members valid disjoint members valid So a boundary that meets the other at all is 21, and only interiors that overlap while the boundaries stay apart is 40. Callers branch on the difference: tilemaker's `buildWayGeometry` throws its fast clip away and redoes it with the robust intersection on 21, and leaves the geometry alone on 40. The existing parity test asserted 40 for the overlapping and edge-touching cases; both measure 21. Updated, and a nested case added to cover the 40 arm that is now reachable on its own.
`intersection`, `union`, `difference` and `sym_difference` took a pair of polygons only, so a caller holding multi-polygons had to decompose them, run the pairs, and re-combine — which is not the same function, and quietly changes the answer when the pieces meet. Boost dispatches every areal Boolean through one overlay whatever the operand arity: `bg::intersection(mp, box, out)` and `bg::difference(mp1, mp2, out)` are the same algorithm as the polygon pair. So the arrangement kernel is now shared, and `*_multi` runs it over the union of both operands' rings. A single polygon is the one-member case. No behaviour change for the existing entry points: `overlay` builds the same `Shape` and segment list it always did, and the split-edge core is byte-identical to what it was. Covered by `multi_polygon_operands`, where the operands overlap across members: two disjoint unit squares against one that straddles the first.
…rsection `is_valid_polygon_with` reported `DisconnectedInterior` for any hole that overlapped the exterior boundary. Boost reports `failure_self_intersections` when they share a *curve*, and reserves `failure_disconnected_interior` for isolated contacts that cut the interior in two. Boost 1.83, exterior (0,0)-(10,10) clockwise: hole fully interior valid hole touching the exterior at one point valid hole sharing a segment of it failure=21 hole touching it at two isolated points failure=32 The inner-vs-inner loop a few lines below already made that distinction; the exterior-vs-inner one did not. Found in tilemaker: a building clipped to a tile edge acquires a hole that runs along the clip boundary, so it hits this on real data. The two codes take different branches there — `buildWayGeometry` redoes the clip with the robust intersection on 21 and only repairs in place on 32 — and the tile came out with two vertices the reference does not have. The existing `polygon_detects_disconnected_interior` case asserted 32 for a hole sharing the left edge; it measures 21. Replaced with the four shapes above, so both arms are covered by a case that reaches them on its own.
…erminant `line_cross_point` solved the two-line determinant, whose terms are products of *absolute* coordinates. On geographic input that is the wrong arithmetic: a polygon spanning 1e-4 degrees at longitude 7.4, latitude 48.7 forms terms around 362 and asks for a residue ten orders of magnitude smaller, so the crossing lands off the line it is supposed to be on. segment (7.4264305,48.7356554)-(7.4263938,48.7355668) against the horizontal clip edge y = 48.7355713 determinant y off by 9.24e-14, and 4.97e-14 off the sloped segment parametric y exact, 3.09e-20 off the sloped segment Split edges that should meet at a shared node then do not, the arrangement is left unclosed, and the traversal strands: `intersection` returned `OverlayError::Unsupported` for a building clipped to a tile edge. The parametric form touches only coordinate *differences* — the size of the geometry rather than of its position — and anchors the answer on the first segment's start, so it stays on that segment by construction. Same point in exact arithmetic, and the same cost. Measured on 17 polygon-against-clip-box cases taken from real tilemaker runs, where the expected output is C++ Boost 1.83's: 14 differed and 2 refused before, all 17 exact after.
Which vertex a ring starts at and which polygon comes first are both observable, and both were falling out of whichever boundary edge the traversal happened to seed from. Boost's are not arbitrary: * a ring begins at a **turn**, the first one in source order; * a ring with no turn was copied whole from one operand and keeps that operand's own starting vertex — not the end the traversal reached it from, which for a hole is the other one, since a hole is walked against its stored direction; * the rings come out in source order, so the polygons do too. Nodes are numbered as the operands are walked, so "first in source order" is just the smallest node index; the tracer now records which nodes are turns (`push_split` marks them, including a crossing that lands on a vertex) and rotates each cycle onto its earliest one. This matters to any consumer that pins a ring's first vertex. tilemaker's `simplify_ring` never removes one, so a rotation of one place shows up in the tile as one extra vertex and one missing. Measured on 27 polygon-against-clip-box cases from real tilemaker runs, compared to C++ Boost 1.83 including ring start and polygon order: 0 of 27 matched before, 25 of 27 after. The two that remain are one case whose two output rings begin at the same vertex, where source order does not separate them.
A union ring begins at a turn — the first one along the first operand's boundary — which `push_ring` decided by node index, the order the nodes happened to be created in. That order is wrong at exactly one place: a turn landing on a vertex is created when the *leaving* segment is split, so a turn on the first operand's own first vertex came out as position zero. Boost normalises the other way. `get_turns` attaches an intersection at a segment endpoint to the segment it **terminates**, so that turn is the *last* position on its ring, not the first. Nodes now carry an `arrival` order built with that normalisation — only the far end of a split counts — and the start turn is chosen by it. Measured against C++ Boost 1.83. Two quadrilaterals sharing an edge, the first given starting at one of that edge's endpoints: before D C B A F E D after A F E D C B A = Boost This is the tenth defect found by diffing the tilemaker port against the C++ it was ported from, and it is what made that port's building outlines differ: `make_valid`'s dissolve merges a self-intersecting outline's two lobes with `union_`, and the merged hole came back rotated three vertices. With this, two of the port's three remaining building differences are gone. Unions whose operands share a *collinear edge* are still not exact — Boost drops the starting turn there when the outline runs straight through it, which is its collinear turn classification and a separate piece of work. 104 of 112 such pairs differ; of the 200 pairs that meet only at crossings or points, 169 are exact and 25 of the rest differ only in the last bits of an irrational crossing. Gate: 1606 tests pass, fmt and clippy clean.
Two more pieces of Boost's overlay, both read out of its source rather than
inferred from its output. Together they take unions whose operands share a
collinear edge from 8 of 112 exact against C++ Boost 1.83 to 89.
**`clean_closing_dups_and_spikes`.** `traverse_with_operation` runs it over
every ring the traversal produces, and it erases the ring's first point while
that point is collinear with its two neighbours, repeating until it is not. A
ring starts at a turn, and where two operands share an edge a turn need not be
a corner — the outline can run straight through the very point the traversal
seeded from. This is why Boost's union of two squares sharing an edge has five
distinct vertices where the obvious answer has six.
Only the *start* is cleaned, which is what makes the behaviour look arbitrary
from outside: the identical straight-through vertex at the far end of the same
shared edge survives. And it has to run once the ring is in its final winding,
because reversing a ring changes which vertex the erasure leaves at the front —
so it moved into `assemble`, next to `orient_ring`, and applies to the rings
the traversal assembled rather than to one copied whole from an operand.
**Turn order is a pair.** `get_turns` walks the first operand's sections in the
outer loop and the second's in the inner, so its turns come out ordered by the
pair. Two turns on the same stretch of the first operand are separated by where
they sit on the *second* — and rotating the second operand moves the answer,
which a single position counter cannot express. `Node::arrival` is now the
segment index per operand, with the fraction demoted to a third key so it
cannot outrank the second operand.
Measured over 312 valid polygon pairs — every rotation of squares and
rectangles sharing an edge in whole and in part, meeting at a corner, crossing,
contained, disjoint, plus radial polygons:
before after
collinear edge (112) 8 exact 89 exact
crossings only (200) 169 exact 173 exact, 26 of the 27
remaining differ only in the
last bits of an irrational
crossing
The 23 that remain share a collinear edge and need what is not here:
`get_turns`' sectionalisation, which groups monotone runs so that turns Boost
treats as tied are not, and its collinear turn classification, which decides
whether a same-direction overlap contributes a turn point at all.
Gate: 1608 tests pass, fmt and clippy clean.
Between two turns Boost copies the vertices of the single operand it is
walking, so a point that operand has no vertex at is passed over. This
arrangement splits a segment wherever anything touches it, so the *other*
operand's corner landing part-way along became a vertex of the traced ring that
Boost never emits.
Edges now carry who runs along them and nodes know whose vertex they are, which
is what the rule needs: along a stretch both operands carry, the one walked is
the first, so a point the first operand has no vertex at goes. Two guards keep
it honest — the point must be collinear with its neighbours, because anywhere
the outline actually turns the point is part of the shape whoever owns it, and
a point the first operand does have a vertex at stays.
Measured over the same 312 valid polygon pairs, against C++ Boost 1.83 and
compared exactly, ring start included:
before after
shared edge, same way (32) 9 exact 20 exact
shared edge, opposite (80) 80 exact 80 exact
no shared edge (200) 173 exact 173 exact, 26 of the 27 remaining
differing only in an irrational
crossing's last bits
Two other formulations were tried and dropped, both of which looked right on
the cases they were reasoned from and measured worse: dropping any point that
is a vertex of exactly one operand (23 differ becomes 32), and dropping the
*entry* of a shared stretch (23 becomes 20, and it loses cases this rule
keeps).
That second one is the signpost for the 12 that remain. They are all
same-direction shared edges, where `get_turn_info`'s collinear handler emits
one turn for the whole overlap and puts it at the far end
(`non_opposite_to_index`), so the near end is not a turn and never reaches the
output. Reproducing that needs `get_turns`' own collinear turn handling, not a
further rule layered over the arrangement; the limit and its current number are
recorded on the function.
Gate: 1609 tests pass, fmt and clippy clean. tilemaker's `scripts/verify.sh`:
16 passed, 0 failed, unchanged.
…ns by section
The previous commit approximated the traversal's vertex handling with a rule
over the finished cycle, and left twelve shared-edge pairs it could not reach.
This replaces the rule with the mechanism, read out of Boost's source: the
traversal appends a *turn* through `append_no_collinear`, which drops any point
the new one now runs straight through, and copies the vertices between two
turns through `copy_segments`, which does no such check. So a shared edge's near
end goes not because anything decided it was interior to a walked segment, but
because the far end of the overlap is a turn collinear with it and takes its
place — while an operand's own straight-through vertex, merely copied, stays.
Two things had to come with it.
`append_no_collinear` and `clean_closing_dups_and_spikes` both judge the point
*before* the one they are given, so which vertex of a straight run survives
depends on the direction the ring runs in. Edges are now oriented with the
filled side on the right, the winding Boost's traversal produces.
And the turn order needed `sectionalize`. `get_turns` partitions each operand
into sections — runs of consecutive segments heading the same way in both
dimensions, capped at ten — and walks the section pairs, so the section pair
outranks the segment index: two turns on one segment of the first operand can
come out in either order depending on which section of the second operand each
one meets. Ring starts are ordered by that pair first, and two rings starting
at the same node by which operand carries the edge leaving it, since `iterate`
tries operation 0 before operation 1.
`Node::is_vertex` was only there for the rule this removes, and goes with it.
Measured over 312 valid polygon pairs against C++ Boost 1.83, compared exactly,
ring start and ring order included:
before after
shares a collinear edge (112) 100 exact 112 exact
meets only at crossings (200) 173 exact 173 exact
differing structurally 13 0
The 27 that still differ all sit on an irrational crossing and differ in its
last bits alone, worst 2.84e-14 across the set.
Gate: 1612 tests pass, fmt and clippy clean. tilemaker's `scripts/verify.sh`:
16 passed, 0 failed.
`difference` does not dispatch the overlay the way `union_` and `intersection`
do: it passes `Reverse2 = true`, so `sectionalize` reads the second operand
through a reversed view and every section and segment index `get_turns` sees
for that operand counts from the other end of the ring. Since those indices are
what order the turns, they decide which turn a result ring starts at — and a
bite taken out of a single edge ties on everything the first operand can say,
leaving the reversal as the whole decision. Ten of the 312 valid pairs started
their ring at the wrong end of such a bite.
The direction reaches nothing else here. The arrangement orients each edge by
which side the result lies on, and the point-in-polygon test that classifies
them counts crossings, so both are indifferent to how a ring was stored.
The remaining three were the order of the polygons, not their contents.
`add_rings` walks the selected rings in `ring_identifier` order, which puts
every ring copied whole from an operand ahead of the traversed ones and orders
the traversed ones by when `traverse` started them — that is, by where
`get_turns` put the turn each one started from. Ordering by the node a ring
starts at got close because nodes are created along the first operand's
boundary, but it cannot separate two rings whose start turns differ only in the
second operand. `TurnOrder` is now the key for both choices, the ring's start
turn and the ring's place in the output, instead of the comparison being
written out once inline and approximated once by node index.
Measured over 312 valid polygon pairs against C++ Boost 1.83, compared exactly,
ring start and ring order included:
before after
difference 36 differ, 13 structurally 25 differ, 0 structurally
union 27 differ, 0 structurally unchanged
intersection 27 differ, 0 structurally unchanged
Every remaining difference across the three operations sits on an irrational
crossing and differs in its last bits alone, worst 2.84e-14. The generator's
self-intersecting inputs are unaffected either way, at 75, 94 and 161 of 600.
Gate: 1614 tests pass, fmt and clippy clean. tilemaker's `scripts/verify.sh`:
16 passed, 0 failed.
`traverse` never sees a ring that nothing crossed. `add_rings` copies it out of its operand through `convert_ring`, which appends nothing and drops nothing, so every vertex it was stored with survives — including one the outline runs straight through on its way back to the first. This arrangement traces such a ring like any other and closed it the way the traversal closes a *traced* ring, through `append_no_collinear`. That function exists to drop the point a new turn now runs straight past, and on the closing append it did exactly that to the last vertex of an untouched ring. Closing is now a plain append unless a turn actually started the ring. Found in tilemaker rather than in the parity set, which has no case where a whole ring survives an operation untouched: the dissolve it repairs polygons with finishes on `difference(outers, inners)`, and a repaired piece that nothing else touched came back a vertex short. Over the 767 `simplify` calls a liechtenstein run makes, the dissolve went from 7 outputs differing against C++ Boost 1.83 to 3, and the whole post-simplify tail — `correct`, the validity check and the repair — from 5 to 3; what is left there is ring order, not content. tilemaker's own gate has liechtenstein identical for the first time. The 312 valid polygon pairs are unchanged at zero structural differences for union, intersection and difference alike. Gate: 1615 tests pass, fmt and clippy clean.
Boost's kernel does not compare coordinates exactly. `math::equals` calls two values equal when they differ by no more than one epsilon of the larger — or of `1`, so values near zero still have to agree absolutely — and `side_by_triangle` opens by calling three points collinear whenever any *two* of them are equal by that rule, before it computes any determinant at all (`side_by_triangle.hpp:150-164`). Every predicate built on the side test inherits it. `CoordinateScalar::tolerant_eq` is that rule, exact for an integer or a rational and relative for a float, and both the side predicate and `remove_spikes` now use it. The side predicate's own documentation already claimed the coincident-pair behaviour; it had the short-circuit only for points that were equal bit for bit. What it changes in practice is what counts as a spike. A hairline whose two ends are four last bits apart at a coordinate of 3540 is one point to Boost, which makes the ring a spike and collapses it to nothing; to an exact cross product it is a sliver with real area, and it survived into a monaco tile that the reference drew as nothing. Measured on tilemaker's own runs, `remove_spikes` now agrees with C++ Boost 1.83 on every call a whole extract makes: 0 of 767 under `config-example`, 0 of 5429 under OpenMapTiles, and 0 of 300 synthetic. The 312 valid polygon pairs are unchanged at zero structural differences for union, intersection and difference — the epsilon only fires where an exact determinant was already down in the last bits. Gate: 1618 tests pass, fmt and clippy clean.
Two things decide which polygon comes out of an overlay first, and neither was right once an operand had more than one ring. **`partition`.** `get_turns` does not compare every section of one operand against every section of the other. It hands both lists to `geometry::partition`, which halves the plane recursively and visits only the pairs that can still meet — and only when *both* lists are longer than sixteen; under that it runs the plain nested loop. The turns land in `m_turns` in the order that walk finds them, `traverse` starts a ring at the first turn it has not used, and `add_rings` emits the rings in that order. Ordering the turns lexicographically by section pair is the same answer for a single small polygon and a different one as soon as the division kicks in, which is why this only ever showed up on multi-polygon operands. `section_partition` is that walk; `Node` carries the rank of its section pair instead of the pair itself. **Ring identity.** A ring no turn lands on is emitted under its own `ring_identifier` — source, then position within that operand — ahead of every traversed ring. The port ordered those by the lowest node on the cycle, which is not the same thing: this arrangement gives two rings that meet at a point the *same* node, so a piece could inherit its neighbour's position and overtake it. Edges now carry the section they came from, and since a section never spans a ring, the lowest section on a cycle names the ring it came out of. Measured against C++ Boost 1.83, compared exactly, ring order included: union / intersection / difference, 312 valid pairs 0 structural (unchanged) the same three over multi-polygon operands, 300 each 1 -> 0 structural the vendored dissolve, over the 767 `simplify` calls 3 -> 0 differ at all a liechtenstein run makes That last one is the whole of `correct(polygon, out, 1E-12)`, per-polygon simplification and repair included, and it is now exact on every call. Gate: 1618 tests pass, fmt and clippy clean. tilemaker's `scripts/verify.sh`: 16 passed, 0 failed, with liechtenstein identical.
`buffer(g, 0)` is Boost's make-valid idiom and `repair_one_polygon` falls back on it when the dissolve gives up. This crate refused it outright, so an outline the dissolve could not fix came back as nothing. It is not a no-op and not a special case. `buffer_inserter` runs its whole pipeline: each side offsets — onto itself, at zero — into an **offsetted ring**, each join adds what it adds, the turns between those rings are found and checked against the original, and what survives is traversed. Two things fall out of the zero: * A **convex** corner adds nothing. `join_miter` returns early when the two offset points coincide, and at zero they always do. * A **concave** corner adds two `buffered_concave` pieces, each contributing the corner again — so that vertex is emitted three times. The winding a ring is stored with is what decides which corners are which: `closed_clockwise_view` is keyed on the ring *type*, not on how the ring is actually wound, so it is the identity for a type that is already closed and clockwise. Normalising the winding first inverts every corner, and closing the ring after reversing it rather than before starts it at the wrong vertex; both were tried and measured. Where the offsetted rings meet nothing, `discard_rings` drops nothing and the traversal has no work: the rings are the answer. That is the case the repair needs, and it is the case this implements. Where they do meet, what survives rests on the winding strategy's verdict for a turn point `get_turns` computed — a crossing part-way along two segments is decided by the last bits of a determinant. Reproducing the verdict without the arithmetic that produced the point was tried and measured (right 132 of 300, wrong 148), so it declines instead. Against C++ Boost 1.83 with the strategies tilemaker passes, over 300 synthetic polygons: **122 answered, 122 exact, 0 wrong, 178 declined** — up from 300 declined. And the outline that sent tilemaker here, an exterior collapsed to a single point around an interior that still encloses something, now comes back identical: twenty-one points, six corners tripled. `buffer_with(polygon, 0)` no longer returns `Unsupported` for a polygon whose rings meet nothing, which is a change to the public contract and to the test that pinned it. Gate: 1621 tests pass, fmt and clippy clean. tilemaker's buildings case is identical for the first time — 28,769 features, no differences.
Stable moved to 1.98.1, which adds clippy::manual_midpoint, detects the unused trait imports, and rewords a trait-obligation diagnostic. None of this is a behaviour change; f64::midpoint matches (a + b) * 0.5 for in-range values.
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…ations The multi-polygon Boolean entry points were added without refreshing the generated table, which CI checks against the pub-use tags.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan