json_read_tree() and json_disconnect() cleared both back-pointers on every
node of the subtree they handed out. Clearing `parser` throughout is
necessary -- the json_pull can be destroyed while the subtree lives on, so
a surviving `parser` would dangle -- but clearing `parent` throughout cost
more than it bought.
`parent` is a non-owning raw pointer, so keeping it cannot form a reference
cycle or keep anything alive; there is nothing to leak. And within a
detached subtree it refers to nodes the caller now owns as a single unit,
so it stays valid for exactly as long as the subtree itself. Clearing it
only made the tree unwalkable upwards, and made json_free() and
json_disconnect() silently no-ops on interior nodes of a detached tree,
since both find a node's owner through o->parent.
So clear `parser` everywhere and clear `parent` on the detached root alone,
which is the one that pointed out of the subtree at a node the parser still
owns. Split the old clear_back_pointers() into clear_parser_pointers() plus
a detach_subtree() wrapper that adds the root's `parent`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
The \uXXXX decoder tested `ch < 0xFFFF` before taking the three-byte UTF-8
path, so U+FFFF itself fell through to the four-byte branch and came out as
F0 8F BF BF -- an overlong, and therefore invalid, encoding of a code point
that fits in three bytes.
check_utf8() only checks that continuation bytes look like continuation
bytes, not that a sequence is the shortest form, so nothing downstream
noticed: a GeoJSON attribute containing U+FFFF put invalid UTF-8 into the
output tile, where a strict consumer would reject it.
Since `ch` is parsed from exactly four hex digits it cannot exceed 0xFFFF
on its own, so after this change the four-byte branch is reached only for a
code point assembled from a surrogate pair, which is the only way to name
one above the BMP.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
The rewrite is meant to be behavior-preserving, but it carries four
user-visible bug fixes that warrant release notes: tippecanoe-json-tool
--extract on a numeric attribute, surrogate-pair decoding, tile-join
reading a non-string tilejson field type, and non-string values in a
directory tileset's metadata.json.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
jsonpull.h said a json_number is 40 bytes; it is 32 (json_object is 24,
and the repr discriminator fits in the base class's tail padding, so the
8-byte union lands at offset 24).
plugin.cpp's parse_feature() said `j` is freed only just before returning
or as jp->root at end of stream, but there is a third json_free(j) at the
bottom of the loop, for a complete Feature whose geometry came out empty.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
json_disconnect() is documented in jsonpull.h as the supported way to
splice a subtree out of the parser's tree and take ownership of it, but
nothing calls it: read_filter() and parse_filter() used to, and now get
the same guarantee from json_read_tree() clearing back-pointers on the way
out. Cover the behavior rather than leave the primitive dead and untested.
The test pins that the subtree is removed from its parent, that the parser
keeps the rest of the tree, and that the detached subtree stays readable
after the json_pull is destroyed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
The `#define _GNU_SOURCE` carried over from jsonpull.c, where it was
needed to get asprintf() declared. g++ already defines _GNU_SOURCE on the
command line for C++ translation units, so redefining it warns:
jsonpull/jsonpull.cpp:1: warning: "_GNU_SOURCE" redefined
Guard the define rather than drop it, so platforms whose C++ driver does
not predefine it still get asprintf() declared.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
dirmeta2tmp() warned about a metadata entry that was not a string/string
pair and then read it as a string anyway. Under the new type-tagged
accessors that trips the assert in json_object::string(); before them it
reinterpreted the node's storage as a char pointer, which segfaulted for
most values. Either way, tippecanoe-decode and tile-join could not read a
directory tileset whose metadata.json had a numeric minzoom or a nested
object, which is common in metadata.json files written by other tools.
Add the missing continue, and cover it in raw-tiles-test.
pmtilesmeta2tmp() handles the same case correctly but read the key with
string() before its own JSON_STRING check, so the assert would have fired
ahead of the check meant to catch a bad key. Hoist the check above the
read. The parser rejects non-string hash keys, so this is unreachable in
practice; the ordering is what makes the check meaningful.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017KNxyHKasyWrWcvre2yK4r
* Only skip polygon cleaning if we are still at very high resolution
* Remove collinear points and clean polygons even at high resolution
* If we truncated but still have the data and need to drop, revive
* Deduplicate by ID even when the duplicate is clipped away
* Test that deduplication works across tile boundaries
* Write out children of a tile revived after its parent truncated
A tile writes the geometry for its children on pass 0 of its zoom level,
and the later passes, which are only retries with new thresholds, must
not write it again. But a tile whose parent truncated its pyramid is
skipped on pass 0, and is only revived on a later pass, once the zoom
has had to start dropping features. Gating on pass 0 meant its children
were never written at all, so a revived tile was always a dead end: it
appeared in the output at ordinary detail with nothing below it, even
though its truncated ancestor still held the full-detail geometry.
Write the children on whichever pass first tiles the tile instead. The
dropping thresholds only ever increase within a zoom, so for a revived
tile that is exactly the pass on which the zoom started dropping.
Also collect the three thresholds into dropping_features(), since
write_tile() and run_thread() have to agree about when truncation is
disabled and when a skipped tile comes back.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Treat dropping by attribute like the other ways of dropping features
--drop-by-attribute-as-needed was added after variable-depth pyramids,
and minattribute never made it into the test for whether a zoom level is
discarding features. A zoom that was dropping by attribute could still
truncate pyramids, so some of its tiles became full-detail leaves while
the rest of the zoom had features dropped out of them, and tiles skipped
because an ancestor had truncated stayed missing.
Unlike the other thresholds, minattribute starts at the infinity on
whichever side is being kept rather than at zero, so dropping_features()
now takes the direction too.
On tests/tl_2022_11_tract at -Z10 -M15000, zoom 11 was dropping by
attribute and truncating two pyramids at the same time; now it truncates
none of them and the tile that had been skipped under zoom 10's
truncation is written out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Delete the merged tile that the deduplication test leaves behind
overzoom-test removes merged-dedup.pbf.json.check but not the
merged-dedup.pbf it was decoded from, so the file was left in the working
tree after every test run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Document what variable-depth pyramids now do to geometry and to dropping
Truncated tiles are no longer left uncleaned: they keep every vertex that
isn't collinear with its neighbors, but their polygons are cleaned so
that overlapping areas are merged instead of stacked. Say so, and say
that dropping features at a zoom level now suppresses truncation for the
whole zoom rather than for individual tiles.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Start minattribute out at the infinity that excludes nothing
dropping_features() reads write_tile_args::minattribute, and the in-class
default of 0 decodes as a threshold that has already been chosen. Every
path assigns it from zoom_minattribute before anything reads it, so this
changes no behavior, but a future one that didn't would silently suppress
pyramid truncation rather than fail visibly.
-HUGE_VAL is the value that excludes nothing for the ascending order that
drop_by_attribute_descending also defaults to, so the two members agree.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Regenerate the drop-by-attribute fixture through the test harness
The Makefile can't be asked for a target whose name contains an =, since
make reads that as a variable assignment, so this fixture was generated by
hand into a scratch directory. The output path ends up in the tileset's
name, description, and generator_options, and tippecanoe-decode is only
passed -x generator, so all three were compared against the harness's
.check.mbtiles path and could never match. make test failed on it.
Regenerated with the same output path the rule uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Restore the -z14 -M25000 variable-depth fixture
This configuration was dropped rather than regenerated when the -z17
-M10000 fixture was added. It still runs, so it was losing a passing
regression test for no stated reason. Regenerated against current
behavior.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Encode the = in the drop-by-attribute fixture name as %3d
A test output name containing an = can't be asked for on the make command
line, because make reads that argument as a variable assignment, so the
fixture couldn't be regenerated through its own rule. Add %3d to the
punctuation escapes that testargs decodes and use it here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
* Regenerate the man page for the README change
The variable-depth pyramid option's description changed, and man/tippecanoe.1
is generated from README.md, so the committed page no longer matched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KewFM6XCt5W9QBZkTZDjCp
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Fix variable-length-array and uninitialized-union compiler warnings
Clang warns about every variable-length array in C++ (-Wvla-cxx-extension,
on by default), since VLAs are a compiler extension rather than standard
C++. Replace all 57 of them with std::vector, or with std::string for the
mkstemp() template buffers built from tmpdir. Add -Wvla to WARNING_FLAGS so
new ones don't creep back in.
Separately, mvt_value's numeric_value union is 16 bytes wide (the size of
string_value), but both constructors only wrote the 8 bytes of the member
they were setting, leaving the rest indeterminate. The implicit copy
constructor copies the union as a whole, so copying any non-string value
read uninitialized bytes, which GCC reports as
mvt.hpp:83:8: warning: 'v.mvt_value::numeric_value. ... .len' may be
used uninitialized [-Wmaybe-uninitialized]
Give string_value, the widest member, a default member initializer so the
union's full width is initialized however it is later used.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
* Fix remaining float-conversion and format-truncation warnings
Clang's -Wimplicit-const-int-float-conversion flagged two comparisons
against LLONG_MAX, which is not representable as a double and rounds up
to 2^63.
In serial.cpp this was a real latent overflow, not just noise: the guard
`extent <= LLONG_MAX` was really `extent <= 2^63`, so an extent of exactly
2^63 passed it and then hit `(long long) extent`, which is undefined for
that value and yields LLONG_MIN in practice -- the opposite of the clamp
the else branch intends. Make the bound exclusive so the conversion is
always in range. Requires a polygon area at the very top of the double
range to reach, but the clamp now behaves as written.
In mbtiles.cpp the value is only a stand-in for infinity on its way into
JSON, so cast explicitly; the emitted number is unchanged.
Separately, g++ at -O0 warned that `char abbrev[20]` can be truncated by
"%lld", which is correct: the most negative long long needs 21 bytes with
the NUL. That branch is only reached when point_count < 1000, so it cannot
happen today, but size the buffer to fit rather than rely on that, and
replace the garbled comment about how the size was derived.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
* Clamp the low end of extent before converting to long long too
The upper bound was fixed in the previous commit; the same overflow exists
on the negative side. get_area() returns a signed shoelace area, so inner
rings contribute negatively, and a polygon whose holes outweigh its rings
drives extent below zero. Far enough below and `(long long) extent` is
undefined again.
The bounds are asymmetric, so this is not simply the mirror of the upper
one: LLONG_MIN is exactly -2^63 and converts exactly, so unlike LLONG_MAX
it can be an inclusive bound.
Verified with -fsanitize=float-cast-overflow that the previous form traps
on 2^63 and on doubles just below -2^63, and that this one is clean across
both boundaries, the infinities, and NaN (which falls to LLONG_MAX, as it
did before).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
* Add CHANGELOG entries for 2.81.0 and bump the version
CHANGELOG.md was last updated for 2.80.0 (#361), and version.hpp has not
moved since. Twelve PRs have landed in the meantime with no entry: #365,
#368, #375, #382, #384, #385, #391, #395, #397, #399, #400, and #401.
Document all of them, plus this PR, under a single 2.81.0 heading. They are
not given separate version numbers because none of them was ever released
under one -- version.hpp read v2.80.0 throughout -- so assigning a version
per PR would invent release history. 2.81.0 is the version that will
actually carry them.
Where an unreleased PR was corrected by a later one (#384 by #385, #397 by
#399), the pair is described as the single behavior that ships, since the
intermediate behavior was never in a release.
Minor rather than patch bump: the batch adds command-line options.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
* Review feedback: enforce the union-width assumption, describe both clamp ends
The comment on mvt_value's union claimed string_value is the widest member.
That is true on LP64 (16 bytes against 8) but not on ILP32, where size_t is
4 and it ties with double and long long. The default member initializer still
covers the full union either way, so the fix held, but the justification did
not travel. Replace the claim with a static_assert that checks it on whatever
target is being built, so a platform where it stops holding is a compile
error rather than silently indeterminate bytes. Verified the assert is not
vacuous by widening the union in a scratch copy and watching it fail.
The changelog described only the upper end of the extent clamp. Describe both:
the old guard admitted everything below LLONG_MIN too.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
* Add 2.81.0 changelog entries for the four PRs merged from main
#404, #408, #409, and #410 landed while this branch was open. None of them
bumped version.hpp, so they belong under the same 2.81.0 heading as the rest
of the unreleased work rather than getting versions of their own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D8gsGMjK78TQiCGKTZ2PyR
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Don't write an extra byte when the radix sort writes a bucket directly
radix1() writes out a sorted bucket in two places. merge() writes all but
the last byte of each serialized feature and then appends the byte for the
feature minzoom, since the minzoom is the last byte of the feature. The path
taken when a bucket holds only one feature, or when the recursion has
consumed every bit of the index, instead writes the feature's whole
serialized length and then appends another minzoom byte, which is one byte
more than the feature's length prefix says it is. Everything read from the
geometry afterward is then misaligned by a byte.
--prefer-radix-sort lowers the memory limit to 8K so that this code gets
exercised, and any bucket that has to be written directly is enough to
desynchronize the stream, so it fails on several of the existing test
inputs:
$ ./tippecanoe -q -f -o out.mbtiles -z4 -aR tests/ne_110m_ocean/in.json
wrong length decoding feature: used 10, len is 33
Write one byte less here too, as merge() does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu
* Keep the radix sort from recursing forever when it runs out of files
radix1() subdivides a bucket by the next splitbits bits of the index, and
stops recursing once prefix + splitbits reaches the width of the index.
The number of buckets comes from the number of files still available, which
shrinks at every level, so deep enough recursion reaches availfiles / 4 == 1
and therefore splitbits == 0. At that point the recursion consumes no bits
of the index and availfiles stops shrinking, so prefix never advances and
the recursion has no way to terminate.
A splitbits of 0 also makes the shift that chooses a feature's bucket a
shift by the full width of the index, which is undefined. In practice it
leaves the shift count masked to zero, so the bucket number is the whole
index rather than 0, and writing to that bucket runs off the end of the
arrays of open files.
Require at least two buckets so that each subdivision always consumes at
least one bit of the index and the shift is always in range, and don't
recurse at all when the next level would not have enough files to split
with: sort that bucket in memory instead, even though it is larger than
the memory limit asked for, since that is the only way left to get it
sorted.
--prefer-radix-sort, which lowers the memory limit to 8K so that this code
gets exercised, segfaults on tests/feature-filter/in.json without this:
$ ./tippecanoe -q -f -o out.mbtiles -z0 -aR tests/feature-filter/in.json
Segmentation fault
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu
* Check that the radix sort and the in-memory sort agree
The result of a sort shouldn't depend on how the sort was performed, so
rather than checking the sorted output against a committed copy of it,
check that --prefer-radix-sort, which lowers the memory limit to 8K to
force the radix subdivision to recurse, produces the same tiles as sorting
in memory. Nothing new has to be kept up to date, and the comparison holds
regardless of how deeply the subdivision recurses on a given machine, which
depends on how many files it will let us open at once.
What sends the sort down the paths that are otherwise almost never taken is
the shape of the input rather than the size of it, so two small inputs are
generated for the purpose: several well-separated features that are each
too big to sort in memory, which are each written out as a bucket of their
own, and many features at one location, which have to be subdivided until
there are no index bits left. Between them and tests/feature-filter, all
three of radix1()'s branches are covered, including sorting in memory
because there are no files left to subdivide with.
Both of these inputs fail without the two preceding commits, and every
input here failed before them.
The whole target runs in about ten seconds.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu
* Record what the shift by the full index width actually did
Say in the comment that masking the shift count to zero makes the bucket
number come out as the whole shifted index, so the writes go somewhere
past the end of the arrays of buckets, rather than only that the shift is
undefined.
Also correct the note on the test: --prefer-radix-sort sets the memory
limit to 8K, but radix() halves it again, so the subdivision is working
against 4K.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wLk2itWETPBAS9a9yE8zu
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Generate the man page with go-md2man instead of md2man-roff
md2man-roff is distributed only as a Ruby gem -- it is in neither Homebrew
nor apt -- so in practice nobody has it installed and man/tippecanoe.1
drifts away from README.md. It was stale again as of #400: the man page
still had the dead All Streets link that commit fixed.
Switch to go-md2man, the maintained Go port of the same converter (it is
what Docker, podman and runc use). It is packaged as a single static
binary for Homebrew, apt, Fedora and Alpine, and it renders inline code as
bold the same way md2man-roff did, so the man page still reads the way it
used to.
It also emits valid roff, which md2man-roff did not. `mandoc -T lint` goes
from 621 errors and warnings to 1 (an empty .TH date, left empty on
purpose so that generation stays reproducible). 590 of those were
`invalid escape sequence: \fC`, from md2man-roff wrapping every inline
code span in `\fB\fC` -- `\fC` is not a font escape.
md2man-roff was losing content, too:
README: 1/(2^32) of the size of Earth
md2man-roff: 1/(2 of the size of Earth
go-md2man: 1/(2^32) of the size of Earth
README: '{"attr": "operation", "attr2": "operation2"}'
md2man-roff: '{"attr": "operation", "attr2", "operation2"}'
go-md2man: '{"attr": "operation", "attr2": "operation2"}'
Prepend a title block and a NAME section during generation rather than
adding them to README.md, where they would render as noise on GitHub.
The man page had neither, so its header rendered as "tippecanoe()" with no
section, and `man -k tippecanoe` and `whatis tippecanoe` found nothing.
It now renders as TIPPECANOE(1) and is indexed.
Finally, add a CI job that regenerates the man page and fails if the
committed copy differs, so a README edit that needs `make docs` gets
caught rather than sitting stale until someone notices. This is what
makes the missing-tool problem stop mattering: contributors no longer
need go-md2man installed to keep the man page current, since CI will
tell them when it needs regenerating. The go-md2man version is pinned
there because different versions produce different roff for the same
input.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015B6PcMbNY779iaczo6S6Pu
* Strip the redundant blank lines go-md2man puts between paragraphs
go-md2man separates paragraphs with a blank line as well as a .PP macro.
A blank line is itself a break in roff, so the two together double-space
the page: every paragraph was followed by two blank lines rather than one.
md2man-roff did not do this, so it showed up as a regression -- the source
went from 13 blank lines to 200.
Filter them out after generation. Blank lines inside .EX and .TS blocks
are kept, since there they are part of the example or the table rather
than spacing around it; that is all 13 of the ones md2man-roff emitted.
The rendered page loses 186 blank lines and the source loses 187, with
byte-identical non-blank output under both groff -t -man and mandoc.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015B6PcMbNY779iaczo6S6Pu
* Decouple the man page from version.hpp, and name the first section
Review feedback on #408.
Making man/tippecanoe.1 depend on version.hpp turned the docs job into a
hard CI failure on any release commit that bumps the version without
regenerating -- #406, which is open and moves version.hpp to v2.81.0
without touching the man page, would have tripped it as soon as either
merged. The only thing the dependency bought was the version in the page
footer, so every release would have had to regenerate the whole file to
rewrite that one line, gated by CI. Drop it: the source field is now just
"tippecanoe", and the page depends on README.md alone.
Separately, README.md's own title heading became the second .SH, directly
below the NAME section this branch adds, so the page opened with a stray
"tippecanoe" section. Rename it to DESCRIPTION, which is where that text
belongs and what a reader expects after NAME.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015B6PcMbNY779iaczo6S6Pu
---------
Co-authored-by: Claude <noreply@anthropic.com>
* docs: correct README statements that don't match the code
Cross-checked README.md against the option tables in main.cpp,
tile-join.cpp, decode.cpp, jsontool.cpp and overzoom.cpp, plus
options.hpp for the -pX/-aX letter assignments.
Incorrect:
* -aD and -aS were swapped. options.hpp assigns 'D' to
A_COALESCE_FRACTION_AS_NEEDED and 'S' to
A_COALESCE_DENSEST_AS_NEEDED, the opposite of what was documented.
* --limit-base-zoom-to-maximum-zoom was given as -Pb. It is a
prevent flag (P_BASEZOOM_ABOVE_MAXZOOM = 'b'), so it is -pb; -P
is --read-parallel and takes no letters.
* --retain-points-multiplier referred to --tile-size-limit, which
is not an option. The limit it extends is --maximum-tile-bytes.
* The dot-dropping description said tippecanoe "drops 1/2.5 of the
dots for each zoom level above the point base zoom". It keeps
1/2.5 of them, at zooms below the base zoom (prep_drop_states
sets interval only where i < basezoom).
* The default tileset name was given as "file.json". make_metadata
sets both name and description from the output file or directory
name.
* tile-join -r/--read-from was described as a "list of input
mbtiles"; it names a file to read that list from, one per line.
* tippecanoe-decode's -I and -F were given as --integer and
--fraction. Those work only as getopt abbreviations; the real
names are --integer-coordinates and --fractional-coordinates.
* Development notes said C++11 and suggested g++-5. The Makefile
builds with -std=c++17.
* Malformed references: "-quiet" and "no-simplification-of-shared-nodes".
Undocumented options now covered:
* tippecanoe: -aa/--keep-point-cluster-position,
--preserve-multiplier-density-threshold, -H/--help, the count
operation for --accumulate-attribute, and the
point_count_abbreviated cluster attribute.
* tile-join: -O as the short form of --overzoom, -q/--quiet,
--exclude-all-tile-attributes, --exclude-all-tile-geometries.
* tippecanoe-decode: -y/--include, -x/--exclude-metadata-row.
* tippecanoe-overzoom: -x/--exclude, --exclude-prefix, -J,
-S/--line-simplification, --tiny-polygon-size,
--deduplicate-by-id, --no-tile-compression, -t/--source-tile,
-o/--output, and the long names for -b, -d, -y, -j, -m and -E.
Also noted that CSV latitude/longitude columns are matched
case-insensitively as substrings, added file.csv to the usage
synopsis, and explained the -a/-p letter-bundle syntax that the
short forms throughout the document rely on.
Every newly documented flag was run against a built binary. The
man page is regenerated from README.md per the Makefile rule; that
also picks up the All Streets link fix from #400, which had not
been regenerated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MvSCpD1yQZhRT5iMU9yufQ
* Hide --unidecode-data from the generated usage messages
The option has done nothing since 533e000 removed the only caller of
unidecode_smash(), so listing it advertises behavior the tools don't
have. Move it after the empty-name entry that ends the usage listing,
the same place --no-polygon-splitting and the debug options sit, so it
is still accepted but no longer offered.
This is the situation #409 already fixed for tile-join's
--use-attribute-for-id, but it applies to all three tools that take
--unidecode-data, not just tile-join: main.cpp listed it under
"Filtering features by attributes" and overzoom.cpp under "Modifying
feature attributes", both ahead of the terminator.
tile-join's "Modifying feature attributes" heading covered only this
option, so it goes too rather than being left empty. overzoom.cpp had
no hidden group at all, so one is added. In main.cpp and overzoom.cpp
the heading keeps its other options and stays.
strip_usage_headings() copies every entry with a non-zero val, so the
moved option still reaches getopt_long(); confirmed by running each
tool with --unidecode-data and checking it is absent from --help.
make test passes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MvSCpD1yQZhRT5iMU9yufQ
---------
Co-authored-by: Claude <noreply@anthropic.com>
* Generate the usage message of each tool from its long_options
The usage messages of tile-join, tippecanoe-overzoom,
tippecanoe-json-tool, tippecanoe-decode, and tippecanoe-enumerate were
hand-written lists of options that had drifted years out of date, since
nothing tied them to the options that are really accepted. Move the
option-list printing that tippecanoe already does into a shared
print_usage(), and use it in all the tools, so that the message is
derived from the same long_options table that getopt_long() gets and
can't fall behind it again.
The tables now carry section headings, as tippecanoe's does, and the
options that were only reachable by their short names (tile-join's -O,
-b, -R, and -r among them) are listed for the first time.
Also state the non-option arguments the way each tool really treats
them: tile-join takes source tilesets unless --read-from names a file to
read them from, tippecanoe-decode takes a tileset either alone or with a
zoom/x/y, tippecanoe-json-tool reads standard input when no files are
named, and tippecanoe-overzoom's two forms are the ones its argument
parsing recognizes. tippecanoe-overzoom now reports the missing -o
instead of passing NULL to fopen(), and tippecanoe-enumerate goes
through getopt_long() so that it will pick up any options added later.
The shared getopt_string() replaces the identical loop that four of the
tools each had for building the short option string, and strip_usage_headings()
the one for dropping the headings before getopt_long() sees them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016frkRY1xXtiWjxYuCJ8vZY
* Print the usage message when tippecanoe is run with no arguments
Running `tippecanoe` with nothing at all reported the missing output
file, which is true but is not what someone who typed the bare command
needs to know. Check for the empty command line before parsing and print
the general usage message instead, and leave the specific complaint for
the case where an input file was named but an output file wasn't.
To make the message reachable from there, the options table and the
usage printing move out of main() into a usage() function, as in the
other tools.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016frkRY1xXtiWjxYuCJ8vZY
* Address review: alternation, the dead tile-join option, and --version
Four fixes from review of the generated usage messages:
* `--output` and `--output-to-directory` are one-of, not one required and
one optional, in both tippecanoe and tile-join. A `usage_required_option`
can now name an alternation that it belongs to, and the options in one
are listed together as `(--output=... | --output-to-directory=...)`,
which is what the runtime check enforces.
* tile-join's `--use-attribute-for-id` has had no implementation since
533e000 removed it; only the table entry was left behind, so the option
parsed and then exited with "Unrecognized option". Generating the usage
message from the table turned that into a documented option that doesn't
work, so remove the leftover entry too.
* `--version` was grouped under "Progress indicator", in the options table
and in the README both. Give it a heading of its own now that the
headings are something users see.
* print_usage() left `width` holding the length of the last synopsis line,
and only got away with it because every table so far begins with a
heading, which resets it. Start the option list on a line of its own
instead of depending on that.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016frkRY1xXtiWjxYuCJ8vZY
---------
Co-authored-by: Claude <noreply@anthropic.com>
* wagyu: drop a hole no remaining ring can parent instead of throwing
correct_tree() throws "Could not properly place hole to a parent" when
topology correction leaves a hole whose parent ring was removed (degenerate
input such as stacked duplicate rings from coalesced tiny-polygon
placeholders). That aborts the entire tiling run over one unrepresentable
sliver. Remove the ring and its points instead, matching how other
unresolvable degeneracies are handled.
* Add a regression test for dropping an unplaceable hole
A fuzzer-minimized pair of mutually reversed self-intersecting rings that
makes wagyu's correct_tree fail to find a parent for a hole — the same
failure reported in mapbox/tippecanoe#761. Before the topology_correction
change, running this test exits with EXIT_IMPOSSIBLE via the polygon
cleaning error handler; with it, the clean returns.
Don't prune variable-depth children while a minzoom-gated feature is pending
The minzoom_feature_pending flag from #397 keeps a variable-depth pyramid
subdividing until explicit per-feature minzooms are satisfied, but two gaps
let features still be dropped:
The flag was only set when tippecanoe_minzoom > z + 1, so a feature whose
minzoom is exactly z + 1 never marked the tile pending, even though a leaf
at z carries only z-visible content.
The early-stop commit never consulted the flag: a tile that succeeded in
stopping early inserted itself into skip_children_out unconditionally,
pruning the children the pending feature needed. The flag only inflated
estimated_complexity_out, which the pruning ignores.
Set the flag for any feature excluded below its minzoom, include it in the
early-stop veto, and skip child pruning while it is set. Adds a fixture
covering the minzoom == z + 1 boundary; make test passes with no diffs to
existing fixtures.
--generate-variable-depth-tile-pyramid decides a tile is a leaf once its
geometry fits at full detail, then prunes the tile's entire subtree. The
guard that prevents leafing while deeper content is still pending consults
only feature_minzoom (the automatic dot-dropping zoom); it does not consult
tippecanoe_minzoom, the explicit per-feature minzoom set via the
tippecanoe.minzoom attribute.
So a feature carrying an explicit minzoom deeper than where its region leafs
is excluded from the leaf tile (z < minzoom) while its children are never
generated. It ends up in no tile at any zoom, silently dropped.
next_feature() excludes such a feature and continues without returning it,
so the leaf-prevention guard in write_tile() never sees it. Carry a flag out
of next_feature() when an excluded feature first appears beyond the next
zoom, and feed it into the same estimated-complexity path feature_minzoom
already uses, so the pyramid keeps subdividing down to the feature's minzoom.
The flag only affects estimated_complexity_out, which is written solely under
--generate-variable-depth-tile-pyramid, so builds without that flag are
unchanged. make test passes with no fixture diffs.
Replaces the shared_ptr-based json_object_ptr with a unique_ptr that
has a stateless custom deleter dispatching on json_object::type before
calling the right subclass destructor. Eliminates per-node atomic
reference-counting and the control-block allocation that shared_ptr
required for every node in the tree.
API now distinguishes owning and borrowing pointers explicitly:
- json_read / json_read_separators / json_hash_get return raw
json_object * (borrowed from the parser-owned tree).
- json_read_tree / json_disconnect return json_object_ptr (caller
takes ownership; back-pointers are cleared so the subtree can
outlive the parser).
- json_free / json_context / json_stringify take raw pointers.
- The parser's container_stack holds raw pointers; jp->root keeps
unique_ptr ownership of the most recent top-level value.
Internally, take_from_owner moves the unique_ptr out of whichever
parent vector / hash entry / parser root owned it, which both
json_free and json_disconnect rely on.
In the streaming parsers (parse_feature, parse_layers, the
geojson-loop callback), we are careful to free `j` only after we
have processed a complete Feature: json_read returns each token
as the tree is being built up, and freeing an intermediate node
would splice it out of the surrounding hash and corrupt the
in-progress feature.
Benchmark (tl_2022_us_county.json, -z0 --extend-zooms-if-still-dropping,
median of 5 runs on macOS arm64): 8.5s, vs 10.6s with shared_ptr
and 8.7s on the pre-refactor C baseline.
Co-authored-by: Cursor <cursoragent@cursor.com>
In the C++ port, json_free was just `o.reset()`, which dropped the
caller's reference but left the subtree alive: the parent's vector
slot kept it allocated, and for line-delimited streams the parser's
jp->root co-owned it until the next top-level value started parsing.
That defeated the geojson-loop pattern of calling json_free on each
feature after serializing it, which is supposed to release the
feature so it doesn't sit in memory while subsequent ones are parsed.
Restore the historical "remove this from the tree" semantics by
splicing the node out of its parent (sharing splice_from_parent with
json_disconnect) and clearing jp->root when the node is the parser's
current top-level value, then dropping the caller's reference.
Two unit tests pin this down: a pruning test parses
"[[1, 2], [3, 4], [5, 6]]" element-wise and confirms that calling
json_free on [3, 4] leaves the outer array with just [1, 2] and
[5, 6]; a top-level test uses a weak_ptr observer to confirm that
json_free on the parser's root really destroys the tree.
Co-authored-by: Cursor <cursoragent@cursor.com>
Profiling tl_2022_us_county.json (sample(1) on Apple Silicon) showed
~38% of parse time in allocator work and ~14% in std::string::push_back
during string-token construction. These changes target the low-hanging
fruit from that profile:
- Pre-reserve 2 slots in json_array and 4 slots in json_hash so
coordinate `[x, y]` pairs and typical GeoJSON property maps avoid
the 0 -> 1 -> 2 -> 4 vector-growth chain (and the shared_ptr copies
it incurs).
- Reuse a parser-wide std::string buffer for JSON_STRING tokens
instead of constructing a fresh local std::string per token. The
buffer is cleared (capacity preserved) at the start of each token
and copied into the final json_string, so once it has grown to the
longest string seen it stops reallocating entirely.
- std::move the freshly-created container shared_ptr into the parser
container stack in the `[` and `{` handlers, and move it out of the
frame on the matching `]` / `}`. Each move skips one atomic
inc/dec round-trip per container open and close.
On a tl_2022_us_county.json benchmark (4-iter user-time mean, Apple
Silicon, /usr/bin/time):
- main baseline: ~8.17s
- jsonpull-cpp before these changes: ~10.90s (+33%)
- jsonpull-cpp with these changes: ~9.33s (+14%)
So this commit recovers roughly half of the post-port regression.
The remaining gap is dominated by shared_ptr atomic refcount traffic
on the parse tree and per-node heap allocations, which would require
the larger unique_ptr/arena reworks to address.
Co-authored-by: Cursor <cursoragent@cursor.com>
Covers the `c` vs `ch` bug fixed in the previous commit: parsing
"\uD83D\uE000" (a valid high surrogate followed by a non-surrogate
BMP code point) used to mis-classify U+E000 as a low surrogate and
combine the two units into U+1F400 (F0 9F 90 80). The fixed code
flushes the stale high surrogate as standalone CESU-8 (ED A0 BD)
and then encodes U+E000 normally as EE 80 80. Verified the test
fails under the pre-fix logic.
Co-authored-by: Cursor <cursoragent@cursor.com>
- jsontool.cpp `out()`: route JSON_NUMBER (and anything else non-string)
through `json_stringify` instead of `o->string()`, which now asserts
on a non-string type and would crash `--extract` on numeric attributes.
- geojson.{hpp,cpp} `json_end_map`: take `json_pull_ptr` by reference so
the caller's shared_ptr is released, null-guard before touching
`jp->source`, and clear `jp->source` after delete to avoid a dangling
pointer.
- jsonpull/jsonpull.cpp: low-surrogate range check was comparing the
outer-loop byte `c` instead of the parsed code unit `ch`, breaking
surrogate-pair decoding for some \\uXXXX escapes. Pre-existing bug
preserved across the port.
- tile-join.cpp `handle_vector_layers`: require the field value to have
type JSON_STRING (and the key to be non-null) before calling
`string()`; the previous truthy `type` check would assert on a
non-string value.
Co-authored-by: Cursor <cursoragent@cursor.com>
json_number used to carry three parallel 8-byte fields (a double plus
both a 64-bit unsigned and a 64-bit signed slot for the large-integer
cases) even though at most one of the integer slots is ever the
canonical value for any given number. Collapse them into a
discriminated union:
enum repr_t { REPR_DOUBLE, REPR_LARGE_UNSIGNED, REPR_LARGE_SIGNED };
repr_t repr;
union { double d; unsigned long long u; long long s; } value;
Callers keep the same read API: number() returns the appropriate
double, large_unsigned() returns the ull (or 0 if not currently stored
that way), large_signed() likewise. Writes go through new set_number /
set_large_unsigned / set_large_signed methods that keep the
discriminator and the union value in sync.
This was prompted by an observation that moving json_type to the end
of the object should shrink things via tail-padding reuse. Empirically
the type-at-end rearrangement saves nothing on its own (every
subclass payload is 8-byte aligned so it can't slot into the 4-byte
tail), but the discriminated-number redesign hits the same idea from
a different direction: adding the 4-byte `repr` to json_number makes
the class non-standard-layout, which lets the Itanium ABI pack `repr`
into the base's 4-byte tail padding at offset 20. The union value
then starts at the natural offset 24, and json_number ends at offset
32 -- a 33% reduction.
Per-node sizes:
json_object (TRUE/FALSE/NULL) 24 bytes
json_number 32 bytes (was 48)
json_string 48 bytes
json_array 48 bytes
json_hash 48 bytes
Numbers dominate real GeoJSON (every coordinate is one), so the net
memory win on a typical parse is substantial.
Co-authored-by: Cursor <cursoragent@cursor.com>
`expect` was only meaningful while the parser was building a container,
and only ever read or written from jsonpull.cpp itself; once parsing
finished it was dead weight on every JSON_ARRAY and JSON_HASH (and
present-but-unused on every primitive too). Move it into the parser's
container stack, alongside the shared_ptr to the container it pertains
to:
struct json_pull::parse_frame {
json_object_ptr container;
json_type expect;
};
std::vector<parse_frame> container_stack;
The base class now only carries data-model state (parent, parser, type).
No external caller depended on `expect`, so no sweep was needed outside
jsonpull.cpp.
This change does not, in itself, shrink any json_object: the 4-byte
`expect` field used to live at offset 20 inside the base, where it was
already being eaten by alignment padding for the 8-byte-aligned first
member of every subclass (std::string, std::vector, double). The win is
in the data model, not the byte count -- the 4-byte hole is still
there, but it is now available for a future subclass whose first member
is small enough to slot into it.
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the parallel std::vector<json_object_ptr> keys / values on
json_hash with a single std::vector<json_entry>, where json_entry is
a small {key, value} aggregate. This still preserves insertion order
(the property the parallel vectors were providing) but removes the
"keep two vectors in lockstep" pattern, and call sites can now use
range-for with structured bindings:
for (auto &[k, v] : o->entries()) { ... }
Side effects:
* sizeof(json_hash) drops from 72 to 48 bytes (one fewer vector
header), matching json_array.
* The keys() and values() accessors on json_object are replaced by a
single entries() accessor returning std::vector<json_entry>&.
* All call sites were swept from the old paired-index pattern
(`o->keys()[i]` / `o->values()[i]`) to entry-based access. Where the
original pattern relied on `nprop = 0` to short-circuit iteration on
a null or non-hash `properties`, the rewrite now guards the loop
explicitly with `if (o->type == JSON_HASH)` so that calling
entries() doesn't trip the asserting downcast.
Co-authored-by: Cursor <cursoragent@cursor.com>
The previous "every member in a struct" layout cost 168 bytes per
json_object, even for JSON_NULL / JSON_TRUE / JSON_FALSE nodes that
have no payload. Splitting json_object into a small base class plus
json_number / json_string / json_array / json_hash subclasses brings
each instance down to just the size of its actual contents:
json_object (base, TRUE / FALSE / NULL) 24 bytes
json_number 48 bytes
json_string 48 bytes
json_array (empty) 48 bytes
json_hash (empty) 72 bytes
Other size wins along the way:
* Drop enable_shared_from_this<json_object> (its embedded weak_ptr
was 16 bytes per node). json_pull now keeps an explicit
container_stack and the parser no longer needs to resurrect a
shared_ptr from a raw `parent` walk.
* Remove the unused `refcon` slot from the string variant.
* No virtual destructor: shared_ptr keeps the deleter from the
original std::make_shared<json_xxx> call, so destroying a
shared_ptr<json_object> still runs the right subclass dtor.
The base class exposes type-tagged accessors (o->string(),
o->number(), o->array(), o->keys(), o->values(), o->large_signed(),
o->large_unsigned()) that assert the type matches and downcast to
the appropriate subclass storage. All call sites were swept from
the old `o->value.X.Y` field paths to these accessors. A raw-pointer
overload of json_hash_get() replaces the few external uses of
shared_from_this() that survived in geojson-loop.cpp.
Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the manual malloc/realloc/free memory management in jsonpull
with std::shared_ptr ownership. Each json_object now owns its children
through std::vector<json_object_ptr>; raw back-pointers to parent and
parser remain valid by structural invariant and are cleared on
json_disconnect so detached subtrees can outlive their parser.
Strings become std::string, child arrays become std::vector, and the
old union becomes a struct so non-trivial members can coexist while
preserving the existing o->value.xxx access paths.
The old jsonpull.c is replaced by jsonpull.cpp, json_stringify now
returns std::string, and all callers across tippecanoe, tile-join,
tippecanoe-decode, tippecanoe-json-tool, tippecanoe-overzoom and the
unit tests are updated to use json_object_ptr / json_pull_ptr.
Co-authored-by: Cursor <cursoragent@cursor.com>
Enhance fqsort function to accept a temporary directory parameter for file handling. Update calls to fqsort in main.cpp, sort.cpp, sort.hpp, and unit.cpp to utilize the new parameter, ensuring temporary files are created in the specified directory.
* Divide-and-conquer polygon cleaning
* Catch the case where the gap can't be increased further
* Catch the case where we try to keep impossibly many features
* Make label points earlier in the tiling process
* Another case where it could try to drop even after already limiting.
* And do not coalesce on impossibly small geometries
* Add missing return
* Update version and changelog
Claude