Repository navigation
Faster tessellation, accuracy and memory fixes, 2D region boolean fix - #27
Merged
Merged
Conversation
Two fresh tessellations of one body must give the same arrays. For a curved face whose trim needs refinement they do not: the annular sector with a round opening gives 0 equal runs of 20 at both deflections, so the first test fails on this commit. The sector with a raised cutout is a second trimmed curved body; it is stable already and stays as a control. The second test pins the sorted set of triangles of both bodies, so the fix that follows can change the order of the vertices only. It passes on this commit. The constants are compared on aarch64-apple-darwin only, where they were recorded: the last bit of sine and cosine can differ between targets.
… fixed order The refinement of a trimmed curved face gave each new midpoint its vertex number in the iteration order of a hash set. That order follows the hash keys of the standard library, which change with every hash collection the process makes, so one body gave different positions, normals and indices from call to call (in WebAssembly: with the history of the instance). The split edges are now visited in sorted order. The set of triangles is the same as before; only the order of the vertices of such a face is now fixed. No other hash collection of the tessellation is iterated: the others are used for lookup only.
The wrapper is about to tessellate through the kernel object and the cache is about to be keyed by shape. Both must leave every array as it is, so this guard comes first and passes before those changes. For a cuboid, a linear extrusion with a hole, an annular sector with a round opening, a planar subtraction, a placed extrusion and a cuboid with another id and revision, at two deflections: a fresh tessellation of the body, a fresh tessellation of the body parsed from its JSON text, the text entry and the object entry give the same bits in all six arrays and in the achieved deflection, and each result carries the revision of its body. The renamed cuboid gives the hash of the first cuboid. The pinned FNV-1a hashes are compared on aarch64-apple-darwin only, where they were recorded, because the last bit of sine and cosine can differ between targets; the equal-entries checks run on every target. The guard is in the test module of wasm.rs because tessellate_cached is private to that file.
…limit The wrapper tessellates a solid through the JSON text of its own BRep only because the text entry is the one that reads max_triangles. The object entry now takes the same optional options text and reads the limit with the same function, so the wrapper can call it. A missing option keeps DEFAULT_MAX_TRIANGLES, as before, so a caller of today does not change. The object entry parses no text, so the 64 MiB limit of the text parser does not apply to it; one test states that with a body whose id alone is 64 MiB long.
AnalyticSolid.tessellate and exportStl asked the kernel for the JSON text of the body and gave that text back to tessellate_brep, so each call wrote the whole BRep as text, parsed it and validated it two more times. Both now call tessellate on the kernel object they already hold. The arrays are the same: the guard test pins that the text entry and the object entry give the same bits. The signatures of tessellate, retessellate and exportStl do not change. One case does: a body whose BRep text is above 64 MiB was rejected by the text parser and now tessellates, because no text is parsed.
These tests come before the cache change. Eleven fail on this commit, each at its cache-hit assertion, because they state the new rule: - nine "cache_hits_when_only_..." tests: a change of the body id, the revision, a face key or one field of the face provenance gives the same arrays from a fresh tessellation and must give a cache hit; - the cache keeps every shape of one body id under the byte bound (the old rule kept three for one id); - two equal shapes share one mesh, and each result carries the revision of the body that asked. Three pass already and must keep passing: the byte total is exact at the bound, the least recently used mesh goes first and a hit refreshes it, and a mesh above the bound is returned and not kept.
…out a scan The cache key was a hash of the whole BRep text, which holds the id of the body, its revision, the face keys and the face provenance. A body that is built again gets a new id, so it never hit; the cache filled to its 64 MiB bound and each call scanned all entries up to three times. The key is now a hash of what two builds of one shape share: the schema version, the geometry, the topology (of a face: id, surface, sense, trim and shell), the solids, the accuracy, the quality, the deflection and the triangle limit. It leaves out the body id, the revision, the face key and the face provenance. cache_key takes the envelope, the topology and each face apart field by field, so a new field does not compile until someone decides whether it belongs to the key. A hit now can come from another body, so a clash of the hash would give the mesh of another shape. The key is therefore 128 bits: two hashers of the standard library with different fixed prefixes. The entries are a map by key with a recency order and a running byte total: one lookup, one recency update on a hit, evictions from the oldest end until the total is under the bound. The 64 MiB bound stays. A cached mesh can serve bodies of different revisions, so the result that goes to JavaScript carries the revision of the body that asked. cache_tracks_geometry_even_when_revision_is_not_incremented changed in two places. It changed a face key and expected a miss; a face key is a label and no longer in the key, so it now changes the radius, which is what its name says. It counted three entries after six inserts for one body id; the rule "three entries for one body id" is gone because the body id is not in the key, so all six stay under the byte bound.
…anges The "cache_hits_when_only_..." tests guard one direction: a field that is left out of the key must not change the result. Nothing guarded the other one: a field that the tessellation reads could be dropped from the key (for example `accuracy: _`) and no test failed, although the cache would then give the mesh of another shape. Each new test changes one such field of a valid body and requires that the cache gives what a fresh tessellation gives: another mesh and a second entry, or the same error. Six also require that the fresh result changes: the geometric accuracy (the fresh call is an error), a vertex position, a face sense, a face trim, a curve and a surface. For the intersection accuracy no cheap pair of valid bodies gives another result, so that test requires the miss alone. Each case was shown to fail with its field taken out of the key, and only that case; the key is unchanged by this commit.
The guard called the private tessellate_cached as its object entry, and on the cache entry that the text call had just made. It now calls OGAnalyticBrep::tessellate, the method of the wrapper, on a thread of its own, so the object entry computes its mesh in an empty cache. The limit test asked for exactly the 12 triangles of the body, so its result proved nothing. It now also requires a second cache entry, which exists only when the limit is read from the options.
… revision Two invariants are not visible in the code. The key must hold every field that the tessellation reads, and a field may be left out only if it cannot change the result; a person who adds a field has to decide that. And the cache is public while a hit can return a mesh that was made for another body, so the revision inside the mesh is not the caller's: wasm.rs, the only caller, stamps its own.
The first test of the wrapper. It runs on the built package with node:test (npm run test:wrapper, after npm run build) and has no dependency. Four of its six cases fail on this commit, because the default accuracy takes the largest absolute profile coordinate as the size of the body: - a 5 m wall has geometric 5e-8 at the origin and 1.05e-6 at (100, 100); - a slab centred on the origin gets the accuracy of half its size; - a 0.05 m prism at (50000, 50000) cannot be made at all: its accuracy is 5e-4 and the first tessellation rejects the default deflection; - a 1 m cuboid at (3e6, 0, 3e6) with a matrix fails the placement check of the kernel, which takes the norm of the far corner where the wrapper takes the largest component of the frame origin. Two pass already and must keep passing: an explicit accuracy goes through unchanged, and a door cut at (5000, 5000) equals the cut at the origin after the translation.
…reach The default accuracy of a new body took the largest absolute profile coordinate as the size of the body. That is the distance from the origin, not the size, so a wall at 100 m got a geometric tolerance above 1e-6 and one at 40 km could not be tessellated at all. A body at 100 m is not less exact than the same body at the origin. For the kinds that are given by points (linear and arc-edged extrusions, polygon lofts, planar polyhedra) the size is now the largest side of the box of those points in the frame of the body. The other kinds keep their size expression. The coordinate floor follows what the placement check of the kernel computes: the norm, over the world axes, of the largest absolute coordinate of the corners of that box (for the other kinds a cube of the size around the frame origin). It is taken 128 times the machine epsilon, twice the factor of the kernel, so a body that a matrix moves a little still passes that check. Two results change. A body centred on its frame origin had half its extent as size and now has the full extent, so its default tolerance is up to twice the former one. A body far from the origin gets the tolerance of its size where it got the tolerance of its distance. An explicit accuracy option is unchanged.
A wall that curves gently has a large radius and a short arc. The size rule takes the whole circle of an arc edge (centre plus and minus the radius), so a wall of radius 80 m with a sweep of 10 degrees gets the size 160.6 m and the tolerance 1.606e-6, where its own box is 10 m wide. Four new cases; the expected box comes from the end points and the axis extremes that each case names. Three fail on this commit: the gently curved wall near and at (5000, 5000), the sweeps that pass one axis extreme, and the ring written in both directions. The sweep of 270 degrees passes already, because there the box is as wide as the circle.
The box of an arc-edged body took the whole circle of each arc. A wall that curves gently has a large radius and a short arc: with a radius of 80 m and a sweep of 10 degrees its size was 160.6 m and its tolerance 1.606e-6, where its own box is 10 m wide. Before the size rule such a wall, with its centre near the origin, had 8e-7, so the rule made a normal curved wall less exact than it was. An arc now gives its two end points and each axis extreme of its circle that the sweep passes. The convention is the one of the kernel (ProfileEdge::endpoints in analytic/primitives.rs): a point at an angle is centre + radius * (cos, sin), the arc runs from startAngle to startAngle + sweepAngle, and the sign of sweepAngle is its direction.
The case of the short arcs beside an extreme did not see a missing extreme: for a thin ring the largest side of the box is along the chord, and the extreme only widens the other side. With the extremes taken out of the box that case still passed, and only the sweep of 270 degrees failed. The case keeps its bodies under the name of what it does prove. The new case closes an arc of 20 degrees across each of the four axis extremes with a slab 200 m deep behind it, so the side along the axis is the largest and ends at the extreme. Shown with two temporary faults in the box of an arc: without the extremes all four are missed; with the directions of 90 and 270 degrees swapped the case fails as well.
The box of an arc steps through the quarter turns between its two angles. From about 1.41e16 rad the step "quarter += 1" no longer changes the number, so the loop never ends and memory grows; an infinite angle makes the wrapper throw a TypeError where the kernel has its own error. Neither happened before the size rule. Each case makes the body in a process of its own, with a time limit and a small heap, so a loop that does not end is a failure and not a hang of the test run. The case fails on this commit: the four huge angles give no result (the process runs out of memory) and the two infinite ones give the TypeError.
… count The loop over the axis extremes of an arc counted quarter turns. For an angle of about 1.41e16 rad or more, adding one to the quarter does not change the number, so the loop did not end; for an infinite angle the quarter was not a number and the wrapper threw a TypeError. The loop now counts at most four steps from the first quarter, and it runs no step when the first quarter is not finite. The body then goes to the kernel, which accepts it or rejects it with its own error.
…atrix The kernel bounds the two cap faces of an arc-edged extrusion with the whole circle of each arc of the outer loop (analytic/primitives.rs, the loop over loops[0].edges), and the placement check reads those bounds. The wrapper takes its coordinate floor from the exact box of the arc, so for a short arc of a large circle the floor is below what the kernel measures. Two bodies under a matrix that is not the identity: a 5 m chord of a circle of radius 1.6e6 m, and a 0.05 m chord of a circle of radius 3.2e4 m. Both fail on this commit with "placement coordinate precision exceeds geometric tolerance".
…ts outer arcs The kernel bounds the cap faces of an arc-edged extrusion with the whole circle of each arc of the outer loop, and its placement check takes the magnitude of the body from those bounds. The coordinate floor of the wrapper came from the exact box of the arcs, so a short arc of a large circle got a floor below the check and could not be placed. The size of the body still comes from the exact box. Only the reach now widens that box by centre plus and minus radius for each arc of the outer loop, as the kernel does. For a wall of radius 80 m this floor is about 5e-12, so its accuracy does not change.
…urned frame Five faults in the size rule passed every case. Each now has a case that fails with it, shown with the fault put in and taken out again: - a column 0.3 x 0.3 x 3 m gets 3e-8 (the height left out of the box); - a polygon loft and a planar polyhedron get 4e-8 at the origin and at 5000 m (either kind back to the largest absolute coordinate); - an arc wall with its centre at (5000, 5000) in the profile equals the wall moved by its frame (the arc extremes without the centre); - a wall in a frame turned by 45 degrees gets 5e-8 (the size measured along a world axis); - the far cuboid has the floor 128 * EPSILON * hypot(3e6 + 1, 1, 3e6 + 1), not only a world accuracy equal to the local one (the floor with the factor 64). All pass on this commit; the code is unchanged.
The kernel sets no allocator, so the standard one asks the engine for one 64 KiB page at each need. A large model grows the memory thousands of times, and each growth costs more the larger the memory is. Two tests come before the allocator. The unit test states the size of the reserve as a pure function of the memory in pages: one eighth, at least 1 MiB, at most 64 MiB, and no overflow at 65,536 pages on a 32-bit usize. The function does not exist yet, so the test build fails on this commit. The wrapper test boots the built package, keeps 5000 cuboid bodies and counts the growth steps of the memory. On this commit: 828 steps, where the case allows 100. Its three other cases pass already and are guards: the wasm has one memory of its own, not shared and without a maximum; the memory after the bodies is at most one eighth and one step above the 55,508,992 bytes of today; one body needs at most 4 MiB. The module is declared for wasm32 and for tests, so the native build compiles nothing of it outside the unit test.
The case for 65,536 pages cannot fail on a 64-bit host, where the product of the pages and the page size fits a usize in any order of the arithmetic. A page count of usize::MAX overflows on every host unless the function is total, so this case fails for an arithmetic that can overflow, on the machine that runs the unit tests. The function still does not exist, so the test build fails on this commit as before.
The standard allocator of wasm32 asks the engine for one 64 KiB page at each need and never gives a page back. A model of 2560 rooms grew the memory 11,129 times, and in V8 one growth costs more the larger the memory is: 9.4 s of that open. A global allocator for wasm32 now wraps the system allocator. After a call that grew the memory it allocates and frees one block of one eighth of the memory, at least 1 MiB and at most 64 MiB. The freed block borders the top of the heap and merges into it, so the next allocations need no growth. The block has the alignment 8, the allocator's own on wasm32, so it is one plain chunk. This is the first unsafe code of the crate. Each unsafe block is one call into the system allocator: the four entries pass on the arguments of their caller, and the reserve frees the block that it has just got with the same layout. The size of the reserve is a pure function that is total, with a unit test on every target. The entries stay out of line and share one alloc and one dealloc entry; inlined, the wasm was 246 KB larger. When the engine refuses a reserve, a flag stops all further reserves. The allocation of the caller has already succeeded, so nothing fails and nothing is logged; the kernel then grows page by page as before. The reserve is address space: the memory that the process touches does not change. No setting, no dependency, no linker argument.
The gate target_arch = "wasm32" is also true for WASI and Emscripten, where the system allocator is another one and the reserve is not proven. The reserve relies on the allocator of wasm32-unknown-unknown, so the gate now also asks for target_os = "unknown". The build for the browser compiles the same code as before: its wasm has the same bytes.
"At most one eighth more memory" holds only for small allocations. When one request is larger than the free top, the memory grows by the whole request and the old top stays free; if the next reserve does not fit that old top, one more reserve is added. The free top can then hold two reserves, about one quarter of the memory. The case that bounded the 5000 small bodies now says in its title what it bounds. A new case makes the worst pattern in a kernel of its own: 1 KiB blocks up to a growth after 16 MiB, then one block of a reserve and two pages. It requires more than one reserve above the memory without a reserve (5.56 MiB, 1.62 reserves, 25.1 %) and at most two reserves and two pages. The memory without a reserve comes from a small model of the standard allocator, which gives the page count of the build before the allocator exactly for this pattern. With one quarter as the reserve this case fails; with no reserve it fails too. The three memory test files share one boot helper.
The engine refuses a growth beyond 65,536 pages, so the refused reserve can be reached in Node. Large blocks that are never written bring the memory to a few pages below the limit; there the 64 MiB reserve cannot be granted. The case requires that nothing traps, that the last pages come one at a time up to exactly 65,536, and that a small block is still served there. It runs in about 0.1 s and touches about 10 MiB; the 4 GiB are address space. An engine that cannot give 4 GiB of address space skips the case during the first large blocks, where no reserve is refused yet. With a panic in place of the refused branch the case fails. It cannot see the flag that stops further reserves: with or without it the page counts are the same.
…lean Step 0 of the fix, a measurement: no production code changes. Every test of this file is ignored, so the suite is as before. Six tests state what the region boolean must do and fail today: two squares with a gap between the tolerance (1e-6) and the probe reach (1e-4) must stay two regions; a strip thinner than the reach must survive, alone and across a square; no result may be an open chain closed by a straight line; a hole must go to the innermost region that holds it, for two and for three levels of nesting. Three probes print the mechanism: how each edge of two squares 1e-5 apart is classed (the two facing edges are dropped, because the probe point at 1e-4 lands inside the other square), which outer rings hold each hole (the first one in output order takes it), and the map of the union by gap. Run them with: cargo test --test region_boolean_probe -- --ignored --nocapture
The tests of the fix, before the fix. No production code changes here.
Eighteen fail on this commit and three pass.
tests/region_boolean_probe.rs: the probes of step 0 are gone and its
tests are live. All nine fail: two squares with a gap of 2e-6, 1e-5 or
9e-5 at three sizes and three distances; squares under a millimetre; a
thin strip alone and in four positions against a square, in all three
operations; no ring that is an open chain; a hole to the innermost
region, in both operand orders and for three levels; an island 1e-5
inside a hole; an island that the operation clips.
boolean2d.rs: five cases for the argument that a bounded probe is exact.
Three pass already and must keep passing: coincident edges of the two
operands, a zero-area sliver, a T junction. Two fail: a clearance below
twice the tolerance, and the rule that a ring is degenerate when it is
thinner than the tolerance (a ring 4e-6 wide is lost today, a square of
side 3e-4 too).
offset_regions.rs: the same through the function behind the export.
Both fail: the nested hole, and two regions across a gap of 5e-5.
analytic/booleans.rs: the two defects through boolean_brep. Both fail
with an error today: a frame prism united with an island prism
("profile holes 0 and 1 overlap or nest", in one operand order only),
and a profile hole 2e-5 from the edge of its host ("planar boolean
produced a boundary without source ancestry").
region-boolean.test.mjs under test:wrapper: three cases through
booleanRegions2D, all failing: the 5e-5 gap gives no region, the frame
takes the island's hole, and a boundary that cannot be closed comes
back as rings where it must be an error. That input uses an edge of
1 km whose split misses a vertex 0.5 mm from its end.
The 2D region boolean decided the side of a sub-edge with two probe points at min(len/4, 1e-4) from its midpoint. A boundary nearer than that made both probes agree, the edge was dropped, and the open chain that was left came back as a ring. Two squares with a gap between 1e-6 and 1e-4 united to a triangle; a strip thinner than 1e-4 was lost or took its neighbour with it. Four rules change. The reach of the probe is now min(len/4, 1e-4, clearance/2), where the clearance is the least distance from the midpoint to any other sub-edge that is farther than eps. A probe at that reach is at least half the clearance away from every other such sub-edge, so it sees the fill beside this edge alone. The sub-edges within eps of the midpoint are the coincident ones, which the probes class by the windings of both operands, as before. The probe of the self-crossing offset loops (cap 1e-3) uses the same rule. An open chain is an error, RegionBooleanError::OpenBoundary, and never a ring. The 2D functions return a Result. The analytic callers map it to the InvalidTopology text they raise for the same fact; the exports give "Boolean regions produced an open boundary". At the two calls with a material predicate the predicate's own failure is still reported first. A ring is degenerate when its area is below eps times half its perimeter, that is, when it is thinner than eps. The old test compared an area with a length, so every ring under 1e-6 m2 vanished (squares of side 3e-4), and a ring 10 m long and 4e-7 wide survived. Changed at the ring classification, at the loops of a self-crossing ring and at the result filter of offset_ring_variable. A hole goes to the smallest outer ring that contains it, the rule of the curved boolean. A hole that no region contains is an error, OrphanHole, not a silent drop. Before, the first outer ring in output order took it, so a frame took the hole of an island inside it. offset_polyline_regions and offset_polyline_group_regions returned a Vec and now return a Result, so that no caller can drop the error; their existing tests unwrap. No existing test changes its result.
aka-blackboots
changed the title
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.
What changes
Four parts, each with its own tests. No TypeScript signature changes:
dist/index.d.tsis byte-identical.Behaviour changes
RegionBooleanError::OpenBoundary,OrphanHole) where a wrong ring came back before.Result;edge_probe2takes two more arguments..wasmfile is about 55 KB larger.Tests
cargo test: 457 pass. Nine new probe tests of the region boolean were red before the fix.npm run test:wrapper): 26 pass. New files: the accuracy rule, the memory growth, limit and worst case, and the region boolean.Review focus
The 2D region boolean (the last three commits) has had no independent review yet.
Not in this pull request
offset2d.rs, the curved boolean and the 2.5 tree are not changed.