simon-git: puzzles (main): Simon Tatham
Commits to Tartarus hosted VCS
tartarus-commits at lists.tartarus.org
Fri Jul 7 18:21:02 BST 2023
TL;DR:
6b5142a Move mul_root3 out into misc.c and generalise it.
e6cdd70 grid.c: allocate face/edge/dot arrays incrementally.
23d4322 grid.c: add dot coordinates to debugging dumps.
61e9c78 grid.c: new and improved Penrose tiling generator.
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: 2023-07-07 18:21:02
commit 6b5142a7a9b31922d9c7ef505b27c33d551f5016
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=6b5142a7a9b31922d9c7ef505b27c33d551f5016;hp=ad7042db989eb525defea9298b2b14d564498473
Author: Simon Tatham <anakin at pobox.com>
Date: Sun Jul 2 21:22:02 2023 +0100
Move mul_root3 out into misc.c and generalise it.
I'm going to want to reuse it for sqrt(5) as well as sqrt(3) soon.
grid.c | 129 +-------------------------------------------------------------
misc.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
puzzles.h | 1 +
3 files changed, 127 insertions(+), 127 deletions(-)
commit e6cdd70df867f064b0358dc3dba56d2667ab80a5
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=e6cdd70df867f064b0358dc3dba56d2667ab80a5;hp=6b5142a7a9b31922d9c7ef505b27c33d551f5016
Author: Simon Tatham <anakin at pobox.com>
Date: Thu Jul 6 12:50:49 2023 +0100
grid.c: allocate face/edge/dot arrays incrementally.
Previously, the 'faces', 'edges' and 'dots' arrays in a grid structure
were arrays of actual grid_face, grid_edge and grid_dot structures,
physically containing all the data about the grid. But they also
referred to each other by pointers, which meant that they were hard to
realloc larger (you'd have to go through and rewrite all the pointers
whenever the base of an array moved). And _that_ meant that every grid
type had to figure out a reasonably good upper bound on the number of
all those things it was going to need, so that it could allocate those
arrays the right size to begin with, and not have to grow them
painfully later.
For some grid types - particularly the new aperiodic ones - that was
actually a painful part of the job. So I think enough is enough:
counting up how much memory you're going to need before using it is a
mug's game, and we should instead realloc on the fly.
So now g->faces, g->edges and g->dots have an extra level of
indirection. Instead of being arrays of actual structs, they're arrays
of pointers, and each struct in them is individually allocated. Once a
grid_face has been allocated, it never moves again, even if the array
of pointers changes size.
The old code had a common idiom of recovering the array index of a
particular element by subtracting it from the array base, e.g. writing
'f - g->faces' or 'e - g->edges'. To make that lookup remain possible,
I've added an 'index' field to each sub-structure, which is
initialised at the point where we decide what array index it will live
at.
This should involve no functional change, but the code that previously
had to figure out max_faces and max_dots up front for each grid type
is now completely gone, and nobody has to solve those problems in
advance any more.
grid.c | 391 +++++++++++++++++++++-----------------------------------------
grid.h | 15 ++-
loopgen.c | 10 +-
loopgen.h | 2 +-
loopy.c | 185 +++++++++++++++--------------
pearl.c | 22 ++--
6 files changed, 251 insertions(+), 374 deletions(-)
commit 23d4322fec38f8ffac930bc541e08309e1d02f11
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=23d4322fec38f8ffac930bc541e08309e1d02f11;hp=e6cdd70df867f064b0358dc3dba56d2667ab80a5
Author: Simon Tatham <anakin at pobox.com>
Date: Thu Jul 6 18:20:37 2023 +0100
grid.c: add dot coordinates to debugging dumps.
I wanted these in order to try to check whether all the faces of a
grid were being traversed in the right orientation. That turned out
not to be the cause of my problem, but it's still useful information
to put in diagnostics.
grid.c | 4 ++++
1 file changed, 4 insertions(+)
commit 61e9c782487ea982498955b93d1b94921131059e
web diff https://git.tartarus.org/?p=simon/puzzles.git;a=commitdiff;h=61e9c782487ea982498955b93d1b94921131059e;hp=23d4322fec38f8ffac930bc541e08309e1d02f11
Author: Simon Tatham <anakin at pobox.com>
Date: Fri Jul 7 18:16:04 2023 +0100
grid.c: new and improved Penrose tiling generator.
The new generator works on the same 'combinatorial coordinates' system
as the more recently written Hats and Spectre generators.
When I came up with that technique for the Hats tiling, I was already
tempted to rewrite the Penrose generator on the same principle,
because it has a lot of advantages over the previous technique of
picking a randomly selected patch out of the subdivision of a huge
starting tile. It generates the exact limiting distribution of
possible tiling patches rather than an approximation based on how big
a tile you subdivided; it doesn't use any overly large integers or
overly precise floating point to suffer overflow or significance loss,
because it never has to even _think_ about the coordinates of any
point not in the actual output area.
But I resisted the temptation to throw out our existing Penrose
generator and move to the shiny new algorithm, for just one reason:
backwards compatibility. There's no sensible way to translate existing
Loopy game IDs into the new notation, so they would stop working,
unless we kept the old generator around as well to interpret the
previous system. And although _random seeds_ aren't guaranteed to
generate the same result from one build of Puzzles to the next, I do
try to keep existing descriptive game IDs working.
So if we had to keep the old generator around anyway, it didn't seem
worth writing a new one...
... at least, until we discovered that the old generator has a latent
bug. The function that decides whether each tile is within the target
area, and hence whether to make it part of the output grid, is based
on floating-point calculation of the tile's vertices. So a change in
FP rounding behaviour between two platforms could cause them to
interpret the same grid description differently, invalidating a Loopy
game on that grid. This is _unlikely_, as long as everyone uses IEEE
754 double, but the C standard doesn't actually guarantee that.
We found this out when I started investigating a slight distortion in
large instances of Penrose Loopy. For example, the Loopy random seed
"40x40t11#12345", as of just before this commit, generates a game
description beginning with the Penrose grid string "G-4944,5110,108",
in which you can see a star shape of five darts a few tiles down the
left edge, where two of the radii of the star don't properly line up
with edges in the surrounding shell of kites that they should be
collinear with. This turns out to be due to FP error: not because
_double precision_ ran out, but because the definitions of COS54,
SIN54, COS18 and SIN18 in penrose.c were specified to only 7 digits of
precision. And when you expand a patch of tiling that large, you end
up with integer combinations of those numbers with coefficients about
7 digits long, mostly cancelling out to leave a much smaller output
value, and the inaccuracies in those constants start to be noticeable.
But having noticed that, it then became clear that these badly
computed values were also critical to _correctness_ of the grid. So
they can't be fixed without invalidating old game IDs. _And_ then we
realised that this also means existing platforms might disagree on a
game ID's validity.
So if we have to break compatibility anyway, we should go all the way,
and instead of fixing the old system, introduce the shiny new system
that gets all of this right. Therefore, the new penrose.c uses the
more reliable combinatorial-coordinates system which doesn't deal in
integers that large in the first place. Also, there's no longer any
floating point at all in the calculation of tile vertex coordinates:
the combinations of 1 and sqrt(5) are computed exactly by the
n_times_root_k function. So now a large Penrose grid should never have
obvious failures of alignment like that.
The old system is kept for backwards compatibility. It's not fully
reliable, because that bug hasn't been fixed - but any Penrose Loopy
game ID that was working before on _some_ platform should still work
on that one. However, new Penrose Loopy game IDs are based on
combinatorial coordinates, computed in exact arithmetic, and should be
properly stable.
The new code looks suspiciously like the Spectre code (though
considerably simpler - the Penrose coordinate maps are easy enough
that I just hand-typed them rather than having an elaborate auxiliary
data-generation tool). That's because they were similar enough in
concept to make it possible to clone and hack. But there are enough
divergences that it didn't seem like a good idea to try to merge them
again afterwards - in particular, the fact that output Penrose tiles
are generated by merging two triangular metatiles while Spectres are
subdivisions of their metatiles.
CMakeLists.txt | 3 +-
auxiliary/CMakeLists.txt | 1 +
auxiliary/penrose-legacy-test.c | 98 ++++
auxiliary/penrose-test.c | 117 ++--
grid.c | 371 +++++++++----
penrose-internal.h | 289 ++++++++++
penrose-legacy.c | 506 +++++++++++++++++
penrose-legacy.h | 63 +++
penrose.c | 1165 ++++++++++++++++++++++++++-------------
penrose.h | 114 ++--
10 files changed, 2112 insertions(+), 615 deletions(-)
More information about the tartarus-commits
mailing list