An imported model's materials are reported, and its images are written where you say - #335
Merged
Conversation
…are written where you say `asset-import` read geometry and threw away everything else a glTF document said about itself. It reports the materials now, and writes the images into a directory the caller names. `--images <path>` is the asking. Without it nothing is written and everything is still reported, so a caller learns a model carries textures before deciding where they go -- a command that scattered them beside the blob because the input happened to hold some would be writing files nobody asked for. The directory is created, because the flag names a destination for a set of files whose size the caller cannot know in advance; `--out` names one file whose parent they already chose. Each image is written as `image-<n>` after the document's own address for it, not after the name the document gave it: a name in a file is not a thing that should decide where bytes land on disk, and the index is what a material's texture reference points at, so the JSON and the directory can be matched without guessing. The extension comes from the media type, and that is the one judgement this arm makes about one. It is a judgement about a *name*: nothing here decodes anything, a type it cannot name may be perfectly readable by something else, and calling a JPEG `.png` would be inventing a fact the document never stated. `image/png` and `image/jpeg` are the two the format's own schema names; anything else, or nothing at all, is `UnknownMediaType` -- and only ever when `--images` asked for a file to be named. Without the flag the type is still just reported. `gltf::tables` is new beside `gltf::read`: it does the same container dispatch for the material and image tables that `read` does for geometry. Two callers had that dispatch written out already and each had it subtly different; a third copy in the tool was not worth having. Its images own their bytes, which a borrowed form could not -- the parsed document lives inside the call and nothing pointing into it can be handed back. The two commands still compose and neither learns about the other: import writes files, pack collects them. No pack is written here, and no image is decoded here or below. A format that carries no images answers with two empty tables rather than a refusal. Asking an STL for its textures is a fair question with a true answer. Also corrects the mesh crate's own description of how an image's media type is chosen, which still described a comparison that no longer happens.
…under them Six sites the new import code reached and nothing executed, each given a test rather than an exemption: the two filesystem failures (`--images` pointed at an existing file, so the directory cannot be made; and at a directory already holding `image-0.png`, so the file cannot be written), the half of `UnknownMediaType` that fires when a document names no type at all, the arm that carries a table's refusal out of an import whose geometry read perfectly, the `OPAQUE` and `BLEND` alpha modes -- the first fixture only used `MASK` -- and the whole human-readable output arm, which is a second piece of code that would otherwise ship to everyone who does not pass `--json` having never run once. The one exempt block in this file is the determinism emit half, and the import grew the code above it, so all fifty-two of its lines moved by exactly 313. Checked rather than assumed: every old position maps to a reported-uncovered position with that same delta, and nothing else in the block changed.
…odel
The import read a glTF's material and image tables whether or not
`--images` asked for them, and the image reader refuses a `uri` naming a
second file. So a document with embedded geometry and ordinary sibling
`.png` textures -- the commonest shape a real glTF has -- stopped
importing, for a reason that says nothing about whether its geometry is
sound.
Reading a model is what the command is for. A table that will not read is
reported now: `materials`, `textures` and `images` come back null with
`tables_refusal` carrying the refusal's name and its sentence, and the
blob is written. It stays fatal when `--images` was given, because then
the caller asked for the thing that cannot be delivered.
Nothing is written until every image's name is settled. The loop checked
one image and then wrote it, so a document whose third image had a media
type this cannot name left the first two on disk beside a blob -- the
half-written directory the comment beside it claimed was impossible.
A material names a texture and a texture names an image, which is a step
the report skipped: a caller pairing a material to a file by index was
wrong whenever a texture's index was not its image's. `gltf::textures`
reads that table and the envelope carries it. The material report also
dropped a normal map's scale and an occlusion map's strength -- the two
values a default cannot recover once a document has stated them --
while the doc above it argued that nothing is dropped.
`--images ""` named the working directory, and `create_dir_all("")`
succeeds, so it scattered a model's textures wherever the process
happened to be standing. Refused where the other path rules live.
`Format::tables` sits beside `Format::read` and is exhaustive, so the
tool no longer matches on the format enum behind a wildcard and a format
added later has to answer the question before it compiles. Its `None`
means "this format does not state these in this vocabulary", which is
why the fields are null rather than empty for an OBJ: Wavefront's
material model is a different thing, and an empty array would be saying
something false about the file.
`ExternalResource` said a document kept its *geometry* somewhere else,
which was true when only a buffer could raise it. An image raises it too.
The corpus image floor parsed each seed as JSON directly, so every
binary glTF in it was skipped silently and no container seed ever
reached the image table.
…k follows the code `gltf::textures` reads which image each texture draws from, and shipped without a test of its own. Four now: a source that is named, a texture that names none -- `None` rather than zero, because the format leaves `source` optional for an extension to supply and defaulting it would point every such texture at whichever image happened to be first -- an absent table, and the two wrong-kind refusals. The unreadable-tables message had a test that passed `--json`, so its human-readable half had never run. Two output modes are two pieces of code. The one exempt block in the command's source is the determinism emit half, and the code above it moved again, so its fifty-two lines shift by ninety-five. Checked rather than assumed: each recorded line was located by its own text and every one is byte-identical at that offset.
Reading a document's tables copied every image out of it. An image stored in a buffer view borrows the document, so owning it copies -- and nothing in the format says two images must name two views. A document that points a thousand of them at one shared megabyte costs about thirty bytes an entry to write and a gigabyte to hold. Measured: 375 KB in, 1,099 MB allocated, and the curve is quadratic in the input's length, so an eight-megabyte document reaches hundreds of gigabytes. The command took 437 ms and a gigabyte over an import that used to take 5 ms and three megabytes, and exited successfully. This crate already had the rule: amplification is the danger, not allocation. Geometry answers to a ceiling and so does the material table. Images now answer to the same one, counted in bytes rather than entries because one image can be the whole ceiling on its own and a thousand tiny ones are harmless. The copy is also asked for rather than assumed. It was being paid by callers who only wanted to know what a model carries -- on a four-megabyte texture it is 99% of the call's time and 99.9% of its bytes, and the import paid it whether or not any file was going to be written. `ImageBytes` states the choice at the call site, and the import asks to count unless it was told where to put the files. `Stored` replaces the owning half of `Image`: it states its length separately from its bytes, so a report never needs the bytes in order to say how big something is. Two tests could not fail. Both asserted that a refusal's text mentions `--images`, and every parse error prints the usage block, which lists `--images` -- so they held however the guard behaved. They assert the sentence the rule produces now. And the material and image report is compared by value. Every number in it could have been dropped, negated or swapped without a test noticing: metallic read from roughness, an image's length reported as zero, every texture role relabelled, the texture index replaced with a constant.
…ted at the boundary Nothing called `gltf::tables`. The target read the borrowing half and wrote its own container dispatch, so the one path that copies an image was never attacked -- which is how a document that points many images at one buffer view got as far as it did. It is called now, from the top so that a document whose container refuses in the borrowing half still reaches it, and the two modes are held against each other: the same materials, textures and count, and per image the same name, type and length, with an image that kept bytes keeping exactly as many as it measured. Counting is asserted to hold no bytes at all, which is the property the distinction exists for. A seed for the shape, since nothing else in the corpus reaches that arithmetic: four images over one view. The image ceiling gets the boundary test its siblings have. The copy that exactly fills it is allowed, one byte past is refused by name, and a sum that saturates is over the ceiling rather than back under it -- lengths come out of the document, and two near the pointer's width would otherwise wrap to an acceptance. The writer's arm for an image whose bytes were measured rather than kept is gone. The caller asks for bytes and files together, so nothing could reach it, and an arm nothing can run is worse than no arm: it reads as a case that has been handled. The pass builds the names and the bytes together instead. The exempt block in the command's source moved again with the code above it. Located by content and verified line by line at the new offset.
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.
asset-importread geometry and discarded everything else a glTF document said about itself. It reports the materials now, and writes the images into a directory the caller names.--images <path>is the asking. Without it nothing is written and everything is still reported, so a caller learns a model carries textures before deciding where they go. A command that scattered them beside the blob because the input happened to hold some would be writing files nobody asked for. The directory is created, because the flag names a destination for a set of files whose size the caller cannot know in advance — unlike--out, which names one file whose parent they already chose.Named by index, not by the document's name for them. Each file is
image-<n>, after the document's own address for the image. A name inside a file is not a thing that should decide where bytes land on disk, and the index is what a material's texture reference points at — so the JSON and the directory can be matched without guessing.The extension is the one judgement this arm makes about a media type, and it is a judgement about a name. Nothing here decodes anything; a type it cannot name may be perfectly readable by something else, and calling a JPEG
.pngwould be inventing a fact the document never stated.image/pngandimage/jpegare the two the format's own schema names. Anything else, or nothing at all, isUnknownMediaType— and only ever when--imagesasked for a file to be named. Without the flag the type is still just reported, which is what the layer below promises.gltf::tablesis new besidegltf::read, doing the same container dispatch for the material and image tables thatreaddoes for geometry. Two callers already had that dispatch written out and each had it subtly different; a third copy inside the tool was not worth having. Its images own their bytes, which a borrowed form could not — the parsed document lives inside the call and nothing pointing into it can be handed back. A caller wanting to avoid that copy holds the parse itself and callsgltf::images.The two commands still compose and neither learns about the other: import writes files, pack collects them. No pack is written here, and no image is decoded here or below.
A format that carries no images answers with two empty tables rather than a refusal. Asking an STL for its textures is a fair question with a true answer.
Also corrects the mesh crate's own description of how an image's media type is chosen: it still described a comparison that no longer happens.
Local gates:
cargo fmt --all --checkclean,cargo clippy --workspace --all-targets -D warningszero errors and zero warnings,cargo test --workspace264 suites with zero failures, andcargo test -p renew-cli --test cli73 passed including five new integration tests.