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