Fix click/right-click placement landing one sub-cell off from the actual click
A real, reproducible bug reported as 'misplaced sometimes by a few small squares': Coord.as_fraction() centers a sub-cell at x + 0.5 (so a marker drawn at its own coord's exact pixel position round-trips back to the same coord on click), which meant point_to_coord()'s own rounding was landing exactly on a .5 boundary, the single worst case for floating point, tiny representation error from the col/row math upstream could tip round() to either side and silently return a coord one sub-cell off from the one actually clicked. Reproduced with zero pixel math involved at all, just feeding Coord(...).as_fraction() straight back into point_to_coord(), ruling out the zoom/pan refactor or the legend margin as the cause (both were suspected first). Fixed by subtracting the 0.5 offset before rounding, which recovers a value that's supposed to be an exact integer instead of an exact half-integer, round() is robust to tiny float noise around a true integer, just not around X.5. Verified exhaustively (all 20,000 possible coordinates round-trip correctly now, not just a handful of samples, since the original bug was itself float-pattern-dependent) and locked in with a permanent regression test.
This commit is contained in:
parent
084764aa9b
commit
512a0a41b4
@ -182,15 +182,28 @@ def point_to_coord(point: Point) -> Coord | None:
|
|||||||
col = min(max(col, 0.0), 19.999)
|
col = min(max(col, 0.0), 19.999)
|
||||||
row = min(max(row, 0.0), 9.999)
|
row = min(max(row, 0.0), 9.999)
|
||||||
|
|
||||||
x_idx = int(col)
|
# A real, reproducible bug lived here: Coord.as_fraction() centers a
|
||||||
x = round((col - x_idx) * 10)
|
# sub-cell at x + 0.5 (so a marker drawn at its own coord's exact
|
||||||
if x > 9:
|
# pixel position round-trips back to the same coord), which means
|
||||||
x, x_idx = 0, min(x_idx + 1, 19)
|
# the value being rounded here is supposed to land EXACTLY on a .5
|
||||||
|
# boundary, the single worst case for floating point, tiny
|
||||||
|
# representation error from the col/row math upstream (pixel <->
|
||||||
|
# km conversions, zoom/pan, or even just this function's own
|
||||||
|
# subtraction) can tip it to either side of round()'s tie-breaking
|
||||||
|
# rule and silently return a coord one sub-cell off from the one
|
||||||
|
# that was actually clicked (verified directly: reproduced with
|
||||||
|
# zero pixel math involved at all, just Coord(...).as_fraction()
|
||||||
|
# fed straight back into this function). Subtracting the 0.5 offset
|
||||||
|
# BEFORE rounding recovers a value that's supposed to be an exact
|
||||||
|
# integer instead of an exact half-integer, round() is robust to
|
||||||
|
# tiny float noise around a true integer, just not around X.5.
|
||||||
|
n_col = round(col * 10 - 0.5)
|
||||||
|
x_idx, x = divmod(n_col, 10)
|
||||||
|
x_idx = min(max(x_idx, 0), 19)
|
||||||
|
|
||||||
Y = int(row) + 1
|
n_row = round(row * 10 - 0.5)
|
||||||
y = round((row - (Y - 1)) * 10)
|
y_idx, y = divmod(n_row, 10)
|
||||||
if y > 9:
|
Y = min(max(y_idx, 0), 9) + 1
|
||||||
y, Y = 0, min(Y + 1, 10)
|
|
||||||
|
|
||||||
return Coord(X=LARGE_X[x_idx], Y=Y, x=x, y=y)
|
return Coord(X=LARGE_X[x_idx], Y=Y, x=x, y=y)
|
||||||
|
|
||||||
|
|||||||
@ -1,6 +1,6 @@
|
|||||||
"""Regression coverage for solver.py's geometric resolution."""
|
"""Regression coverage for solver.py's geometric resolution."""
|
||||||
from fenigma import solver
|
from fenigma import solver
|
||||||
from fenigma.models import Board, Clue, Coord, Location, TargetType
|
from fenigma.models import LARGE_X, Board, Clue, Coord, Location, TargetType
|
||||||
|
|
||||||
|
|
||||||
def _board_with_spotters(*coords):
|
def _board_with_spotters(*coords):
|
||||||
@ -104,3 +104,24 @@ def test_manual_coord_override_clears_a_stale_note():
|
|||||||
assert target.location.note is not None
|
assert target.location.note is not None
|
||||||
target.coord = Coord("A", 1, 0, 0)
|
target.coord = Coord("A", 1, 0, 0)
|
||||||
assert target.location.note is None
|
assert target.location.note is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_point_to_coord_round_trips_every_sub_cell():
|
||||||
|
"""A real, reproducible bug: Coord.as_fraction() centers a sub-cell
|
||||||
|
at x + 0.5 (so a marker drawn at its own coord's exact pixel
|
||||||
|
position round-trips back to the same coord on click), which put
|
||||||
|
point_to_coord()'s own rounding exactly on a .5 boundary, the worst
|
||||||
|
case for floating point. A tiny representation error from the
|
||||||
|
subtraction it used to do could tip round() to either side,
|
||||||
|
silently returning a coord one sub-cell off from the one actually
|
||||||
|
clicked, this reproduced with zero pixel math involved at all, just
|
||||||
|
feeding as_fraction() straight back into point_to_coord(). Checked
|
||||||
|
exhaustively, not just a couple of samples, since the failure was
|
||||||
|
itself pattern-dependent (only some coords tripped the float
|
||||||
|
rounding the wrong way)."""
|
||||||
|
for X in LARGE_X:
|
||||||
|
for Y in range(1, 11):
|
||||||
|
for x in range(10):
|
||||||
|
for y in range(10):
|
||||||
|
c = Coord(X, Y, x, y)
|
||||||
|
assert solver.point_to_coord(c.as_fraction()) == c
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user