A binary glTF reads end to end - #330
Merged
Merged
Conversation
… needs Everything under the layers below works on numbers somebody already extracted: an accessor is six values, a mesh is four arrays. This is where those numbers come from, and it is the only layer that knows the format has a document at all. Almost everything a document says is an index into an array — a primitive names an accessor by number, an accessor names a buffer view, a view names a buffer — so almost everything wrong with a hand-edited or truncated one is a number naming a row that is not there. That refusal carries the table, the index, and how many rows there were. Four refusals wrap another layer's, and that is the type's point: a caller wants to know whether the container was malformed, the JSON was, an accessor's arithmetic was, or the geometry was, because those send them to four different places. A wrapping variant answers with its own name and not the inner one's; the inner refusal's numbers survive in the message. A buffer that is not the container's own is refused rather than fetched. Reading a second file would mean opening one, which this crate does not do; decoding an embedded one needs a decoder that does not exist yet. Returning the part of a model this can assemble would be returning half a model without saying so. The shape a name spells joins the component-type table, beside the names rather than with whichever caller read the string, so a second copy of the list cannot drift from the first. A matrix shape reaches it and is refused by name instead of falling into a missing case. A shape name is decoded rather than compared raw, because an escaped one is legal JSON and spells the same shape; refusing it for its spelling would be inventing a rule the format does not have.
The tables are numbers; this turns a document's indices into validated streams and hands them to the layer that assembles geometry. The interesting places are the crossings. Resolving an accessor is one function rather than three call sites, because that is where the stride crosses. The format puts `byteStride` on the view, since interleaved attributes share it, and the arithmetic wants it on the accessor — so a reader that copied it at each attribute would have three chances to forget one. There is a test with a neighbour's bytes between two positions, and probing the mutation that drops the stride reddens exactly that test and nothing else. An index naming a row that is not there carries the table, the index and the count, at every level: an accessor past the accessors, a view past the views, a mesh past the meshes. A primitive with no `mode` is a triangle list, by the format's own default rather than by whichever branch happens to run. A reader that refused one would reject most of the files in the world. Each layer's refusal arrives wrapped in the name of the layer that made it, with its numbers intact: an accessor asked for ninety-nine positions in thirty-six bytes says so, and says it as an accessor's refusal rather than as a document's. The suite compares coordinates by bits. Reading a document does no arithmetic on one: it takes four bytes out of the chunk and puts them in an array, so anything but an exact match is a value that came from somewhere other than where the fixture put it.
The scene walk composes each node's transform with its parent's, places every primitive of every mesh it names, and joins them. With this a container goes from bytes to geometry without anything else being asked of the caller. The walk cannot hang, by construction: every node is entered at most once, checked before its children are pushed. That guard is the whole content of the cycle refusal, and it is checked here rather than left to a fuzzer because a hang is the one failure a harness cannot report — a seed carrying a cycle would wedge it instead of failing it. Probing the mutation that removes the guard does not merely redden a test; the run stops making progress and a timeout ends it, which is the failure the guard exists to prevent, reproduced. It catches two faults with one rule, and both are invalid: a node that is its own ancestor, and a node two parents claim. The hierarchy is a strict forest, so a second visit is wrong either way. A node's transform is a matrix or the three parts, and the parts compose in the order the format states: scale, then rotate, then translate. There is a test reading the same geometry both ways and comparing. A document with no scenes is refused, because the format's own reading of one is a library of entities rather than a model, and a caller asking for geometry is asking it the wrong question. The refusal census was written when only the tables existed, and four of its arms explained why a refusal could not be reached from them. Every one of those reasons went stale the moment this entry point existed, and the wildcard-free match is what said so: adding the cycle refusal stopped the file compiling until the list was read again. All nine are reachable now, and each has a provocation.
The arm that found a binary glTF said its geometry had no reader. It has one now, so detection hands a caller geometry the same way it does for every other format here, and `asset-import` writes a blob from a `.glb` without anything else being asked. The refusal it can return carries the whole layered answer: which of the container, the document, an accessor or the geometry was at fault, and that layer's own numbers. That is one variant on the shared enum rather than thirty — folding three vocabularies in would have meant every reader's refusal census explaining thirty things it cannot reach, where one variant costs five arms and keeps each layer's words where the layer is. It is boxed, and the box is not an optimisation: the inner type carries a geometry refusal of its own, so the two would otherwise be infinitely sized. The recursion is the shape of the thing — a document-shaped format failing to be geometry can fail as geometry, several layers down. Two tests asserted the behaviour this replaces, and both said so in their names. One was written when the container was recognised and unread; the other checked that the command reported a missing reader rather than a broken file. The first half of each still holds and is kept: the container is recognised, and a `.glb` no longer reaches the fallback to be reported as a truncated model. One fixture in the new command test was thirty-three bytes where a chunk length must divide by four. The container layer caught it and named itself, which is the layering working; the fixture is padded and says why.
The one target in this work whose input needs no encoding invented for it. A container is a file: the generator writes files, the target reads them, the merge gate reads the same ones. The accessor target had to carry six parameters in a head because an accessor is not a file; this is what it looks like when the layer under test takes bytes. Nineteen seeds reaching eight outcomes. Six of them read, because six layers have to agree before the reader gets to the arithmetic worth testing — a scene naming a node naming a mesh naming a primitive naming an accessor naming a buffer view, every index landing on a row that exists and every length agreeing with the chunk it addresses. A random walk essentially never arrives there, so the seeds start inside. No seed carries a node hierarchy with a cycle, and that is deliberate. The reader refuses one by entering each node at most once, but if that guard were removed a cycle seed would make the fuzzer and the merge gate stop making progress rather than fail, and a stall is the one outcome neither can report. Probing that mutation locally did exactly that: one test failed, the next stopped, and a timeout ended the run. The cycle is pinned by a deterministic test beside the crate, where a wedged run is a failed test instead of a silent one. The gate asserts what a refusal cannot reveal: geometry that comes back is whole triangles, holds coordinates a bounding box can bound, and has optional arrays whose lengths match the geometry beside them. A reader returning one normal for two triangles would have answered rather than crashed, and everything above it would then read past the end.
The removability cell for the JSON reader was written while nothing depended on it, with a comment saying the first crate to take an edge should discover the obligation from a red lane there rather than from nobody. That is what happened: the mesh readers took one to read the document inside a container, and the cell went red for a list that was no longer upward-closed. The mesh crate brings the command-line tool with it, so both names join the exclusion, and the comment now records that the prediction came true. Four of the uncovered lines were error paths nothing provoked: a normalisation flag that is not a boolean, an attribute index that is not a number, a scene selector that is not one, and a root node that is not one. Each is a document refusal, and each is now a fixture — a reader that took any of them on trust would be indexing a table with something that was never an index. One more test follows a normalisation flag the whole way, from the document to the fraction the accessor layer produces. The fifth was a refusal added without anything asking its name: the fourth time that gap has appeared, in the third enum. Fixed the way it was fixed for the geometry vocabulary rather than by patching another call site — the census asks every variant its name, including the ones this layer cannot reach, so the next one added is covered by a test that already exists.
Three lines the coverage gate named were closing parentheses of multi-line expressions containing `?`. The round before this one I read the same output and reasoned about which branch each line stood for; two of five were right. Reasoning about how a coverage tool maps a region onto a wrapped expression is guessing with extra steps, so these three stop being wrapped expressions: each names the value it computes, which puts the work on a line that plainly runs and reads better besides. Two error paths those `?` operators guard get tests, because they are worth having whatever the tool says about them: a document may name any accessor for an optional attribute and any scene for its selector, and neither number is checked by anything until it is used. Both are the same fault one level apart, and both were untravelled.
The coverage gate named the closing brace of the branch that reads a scene's node list, which is the branch not taken. `nodes` is optional on a scene, so a scene object with no member at all is a legal thing to write — and every fixture had used an empty list instead, which reaches the same answer by a different route. Two documents, one answer, and now both are pinned: a scene naming no geometry has none whether it says so with an empty list or by not having one. The corpus gains the shape too, so the merge gate replays a document nobody had written. A closing brace is one of the few lines a coverage tool's region can be read with confidence: the only region there is the path not taken. That is a different situation from the wrapped expressions in the previous commit, where the region spanned something and guessing was the mistake.
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.
Five commits. With this,
renew asset-import scene.glb --out scene.mshworks: a container goes from bytes to validated geometry, through the layers the previous rows built.The document
GltfErrorwraps four vocabularies, and that is the type's point: a caller wants to know whether the container was malformed, the JSON was, an accessor's arithmetic was, or the geometry was, because those send them to four different places. A wrapping variant answers with its own name; the inner refusal's numbers survive in the message.Almost everything a document says is an index into an array — a primitive names an accessor, an accessor names a buffer view, a view names a buffer — so almost everything wrong with a hand-edited one is a number naming a row that is not there.
NoSuchEntry { table, index, count }carries all three.Resolving an accessor is one function, because that is where the stride crosses. The format puts
byteStrideon the view since interleaved attributes share it, and the arithmetic wants it on the accessor. Three attributes would be three chances to forget one. The test puts a neighbour's twelve bytes between two positions, and the mutation that drops the stride reddens exactly that test and nothing else.The walk cannot hang, and that is the interesting part
Every node is entered at most once, checked before its children are pushed. One rule catches two invalid shapes — a node that is its own ancestor, and a node two parents claim — since the hierarchy is a strict forest.
Probing the mutation that removes that guard did not merely redden a test: the run stopped making progress and a timeout ended it. That is catalogue entry 32's prediction reproduced — "the symptom is a hang rather than an error, which is the one failure the fuzz harness explicitly cannot catch" — and it is why the cycle is pinned by a deterministic test and why no corpus seed carries one. A seed with a cycle would wedge the fuzzer and the merge gate alike.
Fuzzing
The one target in this work whose input needs no encoding invented for it. A container is a file: the generator writes files, the target reads them, the merge gate reads the same ones.
Nineteen seeds, eight outcomes, six of which read — because six layers have to agree before the reader reaches the arithmetic worth testing, and a random walk essentially never arrives. The seeds start inside.
The last structural decision
Format::readreturnsMeshError; the glTF reader returnsGltfError. The alternative to one boxed variant was widening the shared enum with three more vocabularies — thirty-odd variants, each of which five readers' refusal censuses would have had to explain they cannot reach. One variant costs five arms and keeps each layer's words where the layer is. The box is not an optimisation: the inner type carries a geometry refusal, so the two would otherwise be infinitely sized, and that recursion is the shape of the thing.Two tests asserted what this replaces
a_detected_container_says_its_geometry_is_unread_rather_than_malformedandasset_import_says_a_container_has_no_geometry_reader_yetwere honest when written and false now. Both were flagged two firings before this change as owed to it, so a red lane would not be how they were found. The half of each that still holds is kept: the container is recognised, and a.glbno longer reaches the fallback to be reported as a truncated STL.Also
The refusal census in the document suite explained why four refusals could not be reached from the tables; all four reasons went stale the moment the entry point existed, and the wildcard-free match is what said so — adding the cycle refusal stopped the file compiling until the list was read again.
Mat4gainedaffine_inverseandnormal_matrixin the previous PR; this one is where they are used through six layers of document.Verification
Run locally on
x86_64-pc-windows-msvc:cargo fmt --all -- --checkclean,cargo clippy --workspace --all-targets --all-features -- -D warningsclean,renew checkclean,cargo test --workspace263 suites, 2,887 passed, 0 failed.