simon-git: puzzles (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Fri May 21 14:20:47 BST 2021
TL;DR:
985b538 Galaxies: avoid division by zero in draw_arrow().
20a8589 Galaxies: clean up draw/undraw code for dragged arrows.
Repository: https://git.tartarus.org/simon/puzzles.git
On the web: https://git.tartarus.org/?p=simon/puzzles.git
Branch updated: main
Committer: Simon Tatham <anakin at pobox.com>
Date: 2021-05-21 14:20:47
commit 985b538e5346b1aad6d5e09d534fb9ccacfe9235
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=985b538e5346b1aad6d5e09d534fb9ccacfe9235;hp=19aa3a5d4b1a24c747826c6aeb3acaa5673d03b3
Author: Simon Tatham <anakin at pobox.com>
Date: Fri May 21 13:55:35 2021 +0100
Galaxies: avoid division by zero in draw_arrow().
During an interactive drag of an arrow, it's possible to put the mouse
pointer precisely on the pixel the arrow is pointing at. When that
happens, draw_arrow computes the zero-length vector from the pointer
to the target pixel, and tries to normalise it to unit length by
dividing by its length. Of course, this leads to computing 0/0 = NaN.
Depending on the platform, this can cause different effects.
Conversion of a floating-point NaN to an integer is not specified by
IEEE 754; on some platforms (e.g. Arm) it comes out as 0, and on
others (e.g. x86), INT_MIN.
If the conversion delivers INT_MIN, one possible effect is that the
arrow is drawn as an immensely long diagonal line, pointing upwards
and leftwards from the target point. To add to the confusion, that
line would not immediately appear on the display in full, because of
the draw_update system. But further dragging-around of arrows will
gradually reveal it as draw_update rectangles intersect the corrupted
display area.
However, that diagonal line need not show up at all, because once
draw_arrow has accidentally computed a lot of values in the region of
INT_MIN, it then adds them together, causing signed integer overflow
(i.e. undefined behaviour) to confuse matters further! In fact, this
diagonal-line drawing artefact has only been observed on the
WebAssembly front end. The x86 desktop platforms might very plausibly
have done it too, but in fact they didn't (I'm guessing because of
this UB issue, or else some kind of clipping inside the graphics
library), which is how we haven't found this bug until now.
Having found it, however, the fix is simple. If asked to draw an arrow
from a point to itself, take an early return from draw_arrow before
dividing by zero, and don't draw anything at all.
galaxies.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
commit 20a85890d713d9a6b701eee08a76538f0a4334e4
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=20a85890d713d9a6b701eee08a76538f0a4334e4;hp=985b538e5346b1aad6d5e09d534fb9ccacfe9235
Author: Simon Tatham <anakin at pobox.com>
Date: Fri May 21 14:07:13 2021 +0100
Galaxies: clean up draw/undraw code for dragged arrows.
The previous code had multiple bugs. We had completely left out the
draw_update after drawing each arrow; we omitted the usual
precautionary clip() that constrains each arrow draw to the same
rectangle we just saved in the blitter; we re-computed the coordinates
of the opposite arrow at undraw time, instead of saving the
coordinates we _actually_ used after computing them the first time.
And we restored from the two blitters in the same order we saved them,
instead of reverse order, which was harmless at the time (the drawing
happened after both saves), but is generally bad practice, and needed
to be fixed when the code was rearranged to fix the rest of these
issues.
I noticed these issues in passing, while hunting the diagonal-line bug
fixed in the previous commit. These fixes by themselves would have
prevented any persistent drawing artefact as a result of that bug (the
clip() would have constrained the spurious diagonal line to the region
saved by the blitter, so it would have been undrawn again afterwards);
but it's better to have fixed the root cause as well!
galaxies.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
More information about the tartarus-commits
mailing list