Skip to content

[in3Utils, issue #1337] fix isCounterClockWise winding heuristic (not rotation-invariant) - #1338

Merged
fso42 merged 3 commits into
OpenNHM:masterfrom
skerlakanalytics:fix/isCounterClockWise-rotation-invariance
Sep 10, 2026
Merged

[in3Utils, issue #1337] fix isCounterClockWise winding heuristic (not rotation-invariant)#1338
fso42 merged 3 commits into
OpenNHM:masterfrom
skerlakanalytics:fix/isCounterClockWise-rotation-invariance

Conversation

@bskerlak

@bskerlak bskerlak commented Sep 9, 2026

Copy link
Copy Markdown

Summary

isCounterClockWise() decides a polygon's winding direction with a
majority vote on vertex angles measured relative to whichever vertex is
listed first (V0). This heuristic is not rotation-invariant: the
identical polygon can get a different (wrong) answer purely depending on
which vertex is listed first — or, with vertex order held completely
fixed, purely from the polygon's physical orientation in the plane
(arctan2's ±180° branch cut lands in a different place relative to the
vertex angles as the shape rotates). This is not limited to concave
shapes — a plain convex square is affected too.

polygon2Raster() uses the sign of this result to decide whether its
radius tolerance dilates or shrinks a polygon before
rasterizing it. The real release-raster call in com1DFA.py
(radius = sqrt(2), used to build relRaster) is meant to dilate
slightly, so cells whose center sits just outside the boundary are still
included; when winding is misjudged, the tolerance shrinks the polygon
instead. On a real 59-vertex building-footprint release polygon this
reduced 6 legitimate non-empty release cells to 1 — roughly 92% of the
intended release mass silently dropped, no error or warning.

Reproduce

import numpy as np
from avaframe.in3Utils import geoTrans
import matplotlib.path as mpltPath

l_shape = np.array([[0, 0], [0, 4], [3, 4], [3, 2], [1, 2], [1, 0]], dtype=float)

# same polygon, same starting vertex (index 0) -- only the physical
# orientation changes, rotated 100 degrees about vertex 0
rotated = np.array([
    [0.000000, 0.000000], [-3.939231, -0.694593], [-4.460176, 2.259831],
    [-2.490560, 2.607127], [-2.143264, 0.637511], [-0.173648, 0.984808],
])

geoTrans.isCounterClockWise(mpltPath.Path(rotated))  # -> True (WRONG)

# correct, rotation-invariant answer (shoelace formula):
x, y = rotated[:, 0], rotated[:, 1]
np.sum(x * np.roll(y, -1) - np.roll(x, -1) * y) > 0  # -> False
Sweeping this same l_shape through a full 360° turn (vertex order fixed
throughout) finds a contiguous 90° band (91°–180°) where this
happensa quarter of all possible orientations for this one shape.

Fix
Replace the heuristic with the shoelace formula, which is exact and
rotation-invariant for any simple polygon, convex or concave, and needs
no notion of "first vertex" at all:


def isCounterClockWise(path):
    x = path.vertices[:, 0]
    y = path.vertices[:, 1]
    signedArea = np.sum(x * np.roll(y, -1) - np.roll(x, -1) * y)
    return signedArea > 0
Return type is unchanged (numpy.bool_ both before and afterthe old
docstring claimed int but the function never actually returned one).

Tests
Added test_isCounterClockWise_rotationInvariant: for a convex square and
a concave L-shape, asserts isCounterClockWise gives the same answer
across every possible starting vertex, cross-checked against
shapely.LinearRing.is_ccw. This fails on the current implementation for
both shapes (the square included, to make the point that convexity
doesn't protect against this) and passes with the fix.

pytest avaframe/tests/test_geoTrans.py: 26 passed. The one pre-existing
failure (test_snapPtsToLine) is an unrelated pandas/shapely dtype
incompatibility, present identically on master without this change.isCounterClockWise() decided a polygon's winding via a majority vote on
vertex angles measured relative to whichever vertex is listed first. That
heuristic is not rotation-invariant: the identical polygon can get two
different answers depending on which vertex happens to be first, or on the
polygon's absolute orientation in the plane -- reproduced on a plain convex
square, not only concave shapes.

polygon2Raster() uses the sign of this result to decide whether its
`radius` tolerance dilates or shrinks a polygon before rasterizing it. A
misjudged winding silently shrinks a release/entrainment/resistance area
instead of dilating it -- on a real building-footprint release polygon (59
vertices) this reduced 6 legitimate non-empty release cells to 1, roughly
92% of the intended release mass dropped with no error or warning.

Replace the heuristic with the shoelace formula, which is exact and
rotation-invariant for any simple polygon, convex or concave.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

PR Checklist

Please confirm before requesting review:

  • I ran pytest locally without fails
  • I added/updated tests where needed
  • I updated documentation where needed

Confirm before the final merge/rebase into master

  • Commits are sensibly squashed and rebased onto latest master
  • Standardtest run without difference (with recompiled cython code)

isCounterClockWise() decided a polygon's winding via a majority vote on
vertex angles measured relative to whichever vertex is listed first. That
heuristic is not rotation-invariant: the identical polygon can get two
different answers depending on which vertex happens to be first, or on the
polygon's absolute orientation in the plane -- reproduced on a plain convex
square, not only concave shapes.

polygon2Raster() uses the sign of this result to decide whether its
`radius` tolerance dilates or shrinks a polygon before rasterizing it. A
misjudged winding silently shrinks a release/entrainment/resistance area
instead of dilating it -- on a real building-footprint release polygon (59
vertices) this reduced 6 legitimate non-empty release cells to 1, roughly
92% of the intended release mass dropped with no error or warning.

Replace the heuristic with the shoelace formula, which is exact and
rotation-invariant for any simple polygon, convex or concave.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@bskerlak bskerlak changed the title fix isCounterClockWise winding heuristic (not rotation-invariant) [issue #1337] fix isCounterClockWise winding heuristic (not rotation-invariant) Sep 9, 2026
@bskerlak bskerlak changed the title [issue #1337] fix isCounterClockWise winding heuristic (not rotation-invariant) [in3Utils, issue #1337] fix isCounterClockWise winding heuristic (not rotation-invariant) Sep 9, 2026

@bskerlak bskerlak left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small change, but big impact for building polygons

@bskerlak
bskerlak marked this pull request as ready for review September 9, 2026 09:37
@fso42 fso42 self-assigned this Sep 9, 2026

@fso42 fso42 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I have one minor comment...

Comment thread avaframe/in3Utils/geoTrans.py Outdated
@fso42

fso42 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Standardtest ok, apart from known com5

@fso42 fso42 added the confirmed Something isn't working label Sep 9, 2026
bskerlak pushed a commit to bskerlak/AvaFrame that referenced this pull request Sep 9, 2026
@fso42
fso42 merged commit 4075a15 into OpenNHM:master Sep 10, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

confirmed Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants