A glTF material, in the vocabulary glTF states it - #333
Merged
Conversation
There are two material models in this crate now and they are not two spellings of one thing. A material library carries the Phong vocabulary - ambient, diffuse, specular, a specular exponent - because that is what the format stores. A glTF material is metallic-roughness: a base colour, how metallic a surface is, how rough, and the maps that modulate those. Nothing converts between them. Every mapping in circulation is a heuristic: there is no specular exponent that is a roughness, only a formula somebody found acceptable for their renderer, and a reader that applied one would hand back a material no file contained. That is the thing this crate declines to do everywhere else, and the module says so in as many words. A caller wanting one model out of both converts where the conversion can be seen and argued with. Every default here is the format's. A material object has no required members at all, so an empty one is legal and means every default, which is why the type's own default is the answer to it rather than a separate notion of unset. A factor outside the range the schema states is refused rather than clamped, and the refusal names the member. That is the opposite of what this crate does with the material library's specular exponent, and the two differ because the formats do: that range is a convention files exceed, this one is stated by the schema, so clamping would report a material the document does not contain. The value itself is not carried in the refusal - a factor is a float and this vocabulary is compared for equality, so carrying one would cost every refusal in it that property, including the geometry ones that have no materials in them. A primitive's material is reported rather than stored. Geometry and the surface it wears are two facts and the canonical form carries one of them; putting an index into a mesh would change the stored form, its version question and everything that reads it, for a value nothing here can yet use. The reader is attacked from the same bytes as the geometry and separately, because the geometry path never touches a material table: whatever the target accepts is held to every bound the format states, and six seeds carry materials in the shapes that decide the answer. The gate has a floor for them and one for a seed that provokes this layer's own refusals, since a material changes no geometry and no existing floor would have noticed their loss.
Four reviews against this branch, and the checks were green throughout. The reader clamped where its own documentation promises it refuses. A factor's range was checked after narrowing the document's number to the width this crate keeps, and a value one unit in the last place past the maximum narrows to exactly the maximum - so the check could not see it, the document was not conforming, and the caller got the limit with nothing said. The whole argument of this module is that the format's ranges are stated rather than conventional, so they are refused rather than clamped; the ordering inverted that in the one place the difference is invisible. Checked wide, then narrowed. A material, and its shading half, are asked their kind rather than merely their presence. Every member lookup answers absent on a number, a string or an array, so a table of those read as a table of materials that each said nothing, and this reader's own rule then handed back a full default material for a document that described none. An out-of-range factor one array wrapper deep read clean the same way. The material table has the ceiling this crate's amplification rule asks for. Geometry is capped because a megabyte of legitimate input built fifty-eight megabytes of it; a two-byte array element is a whole material, which amplifies harder, and the table had no equivalent. Pairing a mesh with its surfaces is one pass. Asking per primitive walked both tables from the head each time, which is the cost this file converted three other readers away from, and the singular form's own documentation invited exactly that loop. A texture index and a material index are bounded by the table they name. Neither is resolved - this crate does not read the texture table - and the documentation says that rather than implying more. Passing them through unchecked was the one thing this reader's own module doc calls the commonest fault in a hand-edited document. An alpha mode the format does not define has its own refusal, carrying what was spelled. It was being reported as a construct the format defines and this reader does not, which told the caller to convert a file that needed repairing. A colour array is exactly as long as the format states. Reading the first few and dropping the rest hid whatever the tail said, including components that would have been refused. A cutoff is range-checked whatever the mode names, and refused when no mode is named at all, which the format forbids - and a test that enshrined the accepting behaviour is now the test that pins the refusal. Three assertions that could never fire are gone: the layer below already refuses a number that is not finite. The material half of the fuzz target dispatches on the container magic the way the reader does, so the nineteen container seeds it used to walk past now reach it, and the pairing function it never called is attacked too. Smaller: an enum tag is compared without building a string, the table is sized from what was parsed, two members are fetched once rather than twice, glTF ids are stored at the width the format states them in, and the crate root no longer makes one of two material vocabularies the default spelling of the word.
…stable The coverage gate named six lines. Four were the ceiling on the material table, which no test can reach: it sits at two hundred and fifty-six megabytes of materials and nothing builds a table that size. The geometry ceiling beside it had already answered this by being a function rather than an inline comparison - a small check tested at its boundary, so the call site is one line that always runs. The material ceiling has that shape now, tested at the last row it allows and the first it does not. Its documentation claims two probes, and both were run: loosening the comparison lets the exactly-full table take one more row, and removing it lets a table past the ceiling through. A comment claiming a probe nobody ran is the same defect as a test that asserts nothing. The other two lines were fixtures too rich to reach the format's own defaults. The whole-material fixture states a normal scale and an occlusion strength, so the branch that supplies one when the document does not was reached by nothing - and a default that drifted would have been a value this crate invented with nothing to say so. A mesh with no primitives, which pairs with nothing and is not a refusal, had no test either.
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.
There are two material models in this crate now, and they are not two spellings of one thing. A material library carries the Phong vocabulary — ambient, diffuse, specular, a specular exponent — because that is what the format stores. A glTF material is metallic-roughness: a base colour, how metallic a surface is, how rough, and the maps that modulate those.
Nothing converts between them. Every mapping in circulation is a heuristic — there is no specular exponent that is a roughness, only a formula somebody found acceptable for their renderer — and a reader that applied one would hand back a material no file contained. That is what this crate declines to do everywhere else, and the module says so. A caller wanting one model out of both converts where the conversion can be seen.
Every default is the format's, read from the normative schemas rather than recalled. Two are easy to get wrong and are pinned by tests: a material object has no required members, so an empty one is legal and means every default; and
baseColorFactor/emissiveFactorbound each item to0..=1rather than the array.alphaCutoffis bounded below and not above.indexis required on all three texture-info kinds — the normal and occlusion ones inherit it throughallOfrather than restating it.A factor outside its stated range is refused rather than clamped, and the refusal names the member. That is the opposite of what this crate does with the material library's specular exponent, and the two differ because the formats do: that range is a convention files exceed, this one is stated by the schema, so clamping would report a material the document does not contain.
The refusal deliberately does not carry the offending value. A factor is a float and this error vocabulary is compared for equality, so carrying one would cost every refusal in it that property — including the geometry refusals, which have no materials in them at all. The message states the range and the member says where to look.
A primitive's material is reported, not stored. Geometry and the surface it wears are two facts and the canonical form carries one of them; putting an index into a mesh would change the stored form, its version question and everything reading it, for a value nothing in this engine can yet use.
gltf::primitive_materialanswers for the pairing instead.Attacked in the same row, per this tree's rule for anything that reads bytes nobody here wrote. The glTF target reads the material table from the same bytes and separately — the geometry path never touches it — and holds whatever it accepts to every bound the format states. Six seeds carry materials in the shapes that decide the answer, with a corpus floor for them and one for a seed that provokes this layer's own refusals, since a material changes no geometry and no existing floor would have noticed their loss.
The wildcard-free refusal census refused to compile until the new refusal was named, which is the third format or vocabulary in a row it has caught.
Local gates:
cargo fmt --all --checkclean,cargo clippy --workspace --all-targets -D warningsclean,cargo test --workspace264 suites / 2,966 passed / 0 failed,renew checkreports the workspace healthy.