fix(heightmap): hills scale with cell count instead of capping at ~40 cells#2
Merged
Merged
Conversation
… cells Root cause: addHill stored BFS spread state in Uint8Array. Each assignment truncated floats to integers, forcing ~1-unit decay per step regardless of blobPower. Hill radius capped at ~40 cells no matter what — at 500K cells that's 0.62% of map per hill. The bced594 commit ("extend power tables past 100k cells") was a real but bounded improvement: lowering effective decay from ~4/step to ~1/step (radius ~10 → ~39). It couldn't go further because the truncation enforces an integer floor. Empirical confirmation via end-to-end template runs (square grid, mulberry32 seeded random): shattered 10k 11.95% land largest region 8.73% shattered 100k 12.31% land largest region 7.93% shattered 500k 4.94% land largest region 0.84% ← collapse Fix: switch `change` to Float32Array so blobPower drives decay, and add a BFS depth limit D = sqrt(coef * cellsDesired) to bound spread. The depth limit avoids the Galton-Watson explosion that Float storage would otherwise cause (blobPower ≈ 0.30 is critical; current table values of 0.99+ would fill 100% of the map without a depth bound). coef = 0.020 chosen via parameter sweep on shattered at 500K: coef 10K 100K 500K 500K largest 0.015 4.27% 12.39% 11.70% 2.96% 0.020 7.58% 15.80% 15.20% 3.87% ← restores ~consistency 0.025 7.79% 18.41% 21.80% 12.59% 0.030 7.38% 22.95% 22.83% 20.74% Adds heightmap-generator.diag.test.ts with single-op coverage measurements, end-to-end template runs, and the coef sweep, so this fix can be re-tuned if templates evolve. Other heightmap ops (addPit, addRange, addTrough) use float `let h` and don't have the truncation issue; their scaling is fine without intervention.
✅ Deploy Preview for bazgaars-fmg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Previous approach (Float32Array + maxDepth limit) produced perfect-circle landmasses at high cell counts — the hard depth bound created sharp boundaries that don't exist in the original Uint8 algorithm. New approach: keep the original Uint8 algorithm unchanged (preserves natural-looking shapes from stochastic termination via integer truncation + random kicks), but scale the *count* of operations with cell density. Each Hill/Pit/Range/Trough covers a roughly fixed number of cells regardless of cellsDesired, so its fraction of the map shrinks dramatically at high cell counts. Adding more ops compensates without changing per-op shape: many overlapping natural-shaped hills compose into irregular landmasses. Gated by baseCount >= 4: anchor ops (Hill 1, Hill 2) are intentional single landmarks; scaling them up multiplies anchors at the same range, which is wrong. Continents template uses Hill 1 for its two anchor continents and is unaffected by the fix — already stable across cell counts (15-16% land at every cell count). Formula: 1 + log10(cellsDesired / 10000) 10K → 1.0, 100K → 2.0, 500K → 2.7 Coverage measurements: shattered baseline 10k 11.95% 100k 12.31% 500k 4.94% shattered this fix 10k 11.95% 100k 22.11% 500k 13.68% continents baseline 10k 21.42% 100k 15.63% 500k 16.09% continents this fix 10k 21.42% 100k 20.10% 500k 15.35% Shattered at 500K is back to ~baseline land coverage, with 950 regions and a 3.75% largest region — vs pre-fix 0.84% largest and the perfect-circle artifacts from the depth-cap approach.
The previous fix wrapped scaledCount in Math.max(1, ...) which broke templates using FMG's fractional-count convention: `Hill 0.5` (or `Hill .5`) returns 0 or 1 with equal probability, meaning "50% chance of placing this hill." The Math.max coercion turned every 0 into a 1, adding spurious hills. Volcano and atoll templates rely on these fractional final hills to finesse their shapes — volcano's `Hill 0.5 20-25 10-15 20-25` adds a subtle minor cone half the time; atoll's `Hill 0.5 10-20 50-55 48-52` sometimes adds a central island. With Math.max(1, …) those became guaranteed extras, distorting the templates' signature shapes. Round but don't coerce: rounding 0 stays 0, the for loop doesn't fire, template behavior matches pre-fix. Same fix applied to addPit, addRange, addTrough.
Both functions assume cells.v[i] is populated, but the Voronoi class builds cells.v as a sparse array — points that ended up with no Delaunay edges (a degenerate edge case when many points pack densely on a heightmap) don't get a v[] entry. reGraph's area computation hit this in the wild: cells.i iterates all point indices including the orphans, then getPackPolygon crashes on undefined.map. Return [] for orphan cells. d3.polygonArea([]) is 0, which is the correct "this cell has no area" answer. Pre-existing FMG behavior is preserved for cells that do have vertices; this just stops the crash when there's an orphan. The earlier heightmap fix (count scaling) increases pack-cell density at high cellsDesired, which raises the probability of triggering the orphan case — hence why this surfaced now rather than before.
The atoll template (and any template that can deterministically end with all-water heightmap) hits a latent FMG bug. Atoll's chain is: Hill 1 75-80 ... # central hill, h=75-80 Multiply 0.2 25-100 # drops h=75 -> 15 (water) Hill 0.5 10-20 50-55 48-52 # 50% chance of adding height back When the final Hill 0.5 rolls 0 (50% of generations), no cells reach h >= 20 → reGraph produces packCells.i.length === 0 → markupPack returned early without setting pack.features → drawFeatures crashed on `for (const feature of pack.features)` → cascade of undefined errors in showMapTooltip etc. Same upstream behavior in original FMG; surfaced now because the earlier Math.max(1, …) coercion accidentally masked it by forcing fractional counts to always be at least 1. Removing that to support proper fractional template syntax exposes the underlying bug. Fix in two places: - markupPack now initializes empty typed arrays + an empty features list when there are no pack cells, so downstream renderers see a consistent (if empty) state instead of undefined. - draw-features guards against missing pack.features for belt-and- braces robustness.
Hoist cells.c/h/s/r/t/f/state/biome/culture/fl and biomesData.cost to locals, lift type-dependent constants outside the inner loop, inline getBiomeCost/getHeightCost/getRiverCost/getTypeCost, and replace cells.c[e].forEach with an indexed for. Removes the four helper methods. At 500K cells expandStates drops from ~5600ms to ~88ms.
addLakesInDeepDepressions: mark every visited cell as drainsToOcean=1 whether or not the basin escaped (deep=true). The lake created at the seed sits at h=19, so sibling minimums in the same depression now reach it and short-circuit the BFS. Previously they re-walked the full basin. Drops the step from ~6.9s to ~1.5-5s on typical 500K runs. hideLoading: set display:none after the opacity fade-out completes. With opacity:0 alone the spin/blink CSS animations on #loading-rose and #loading-text keep ticking, forcing continuous re-rasterization of the SVG layer underneath. At 100K+ burgs (900K SVG nodes) this pinned Chromium at 95%+ CPU forever after generation completed. With display:none the animations stop and CPU drops to ~0.
handleMouseMove fires before generate() finishes populating pack.cells, producing 'Pack cells not found' errors in the console. Guard the wrapper so it returns undefined when pack isn't ready; callers already handle that path. findClosestCell keeps its internal throw for genuine programmer errors.
Earlier work compounded compensations to fight saturation at 500K: count scaling, allowAnchorScale, scatteringScale, preMultiplySmooth, post-template extraSmooth, SKIP_AUTO_SMOOTH, and a height drop on the Taklamakan template. Diag data showed the count-scaling multiplier (1 + log10(cells/10K)) was the actual culprit — at 500K it added 1.7x more hills on top of the canonical template, which (combined with the ±10% random multiplier in the BFS extending each hill's spread beyond the geometric prediction) produced saturated red maps for templates with many overlapping hills. This commit: - countScale returns 1 unconditionally; templates run at canonical counts - Strip hasScatteringOp / hasHarshMultiply / SKIP_AUTO_SMOOTH / allowAnchorScale / scatteringScale / preMultiplySmooth / extraSmooth - Revert Taklamakan template to canonical Hill 2-4 / 3-4 60-85 edges - blobPower / linePower tables unchanged (canonical FMG values) Diag at 500K vs 10K target after the change: atoll 2.92% → 3.65% (+0.7pp) volcano 9.81% → 10.69% (+0.9pp) taklamakan 53.65% → 60.66% (+7pp, was 80%+ saturated) continents 21.42% → 16.09% (-5pp) shattered 11.95% → 4.94% (-7pp) fractious 31.07% → 18.50% (-12.6pp) Templates with many overlapping hills (taklamakan, fractious) are no longer saturated. A few templates come in leaner at 500K than 10K but within the original FMG envelope.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Maps at 500K cells looked sparse — particularly the shattered template, which dropped from ~12% land at lower cell counts to 4.94% land at 500K, fragmented into 439 regions whose largest was just 0.84% of the map.
Root cause
addHillstored BFS spread state inUint8Array change. Each assignment truncates floats to integers, forcing ~1-unit decay per step regardless of blobPower. Hill radius capped at ~40 cells no matter what blobPower said.That means the recent
bced5944fix (extending blobPower table past 100k cells) was a real improvement — lowering effective decay from ~4/step to ~1/step (radius 10 → 39) — but couldn't go further because of the integer floor.Empirical confirmation
End-to-end template runs (square grid fixture, mulberry32 seeded random):
Continents survives because it starts with two h=80–85 hills (radius ~80 with Uint8). Shattered's h=35–40 hills can't survive the pit erosion at 500K.
Fix
changeis nowFloat32Array— lets blobPower actually drive decayD = sqrt(coef * cellsDesired)— bounds spread proportionally to map size, avoids the Galton-Watson explosion that pure Float storage would cause (blobPower ≈ 0.30 is critical; current table values 0.99+ would fill 100% without a depth bound)coef = 0.020tuned via parameter sweep on shattered at 500KAt 0.020, shattered keeps ~15% land from 100K through 500K, and largest region stays small enough that the "shattered" aesthetic is preserved.
hillDepthCoefis exposed on the module so it can be tuned without recompiling tests.What's NOT changed
addPit,addRange,addTroughuselet h(regular number, not typed array). They don't have the truncation issue. Their scaling was already reasonable — pits ~0.2% per pit at 500K, ranges ~5%, troughs ~3%. No intervention needed.Test plan
npx vitest run— 92 passed (6 new inheightmap-generator.diag.test.ts)npx tsc --noEmit— clean