Repository navigation
Harden the tooling paths and atlas paging, and fix the trim pipeline for 26.x - #99
Merged
Merged
Conversation
The directory flags added in #97 were read from JVM system properties inside static initializers, so they only took effect if the right class was touched first, and the overlay entry point never set them at all. The paths now live in a ToolingPaths value built once per entry point from flags, then system properties, then the repository defaults, and every component takes it as a constructor argument. Along the way: - One AtlasLayout code path. The int-width constructor, the fixed 256px resize helper and the duplicated atlasWidth field are gone; the overlay generator passes an explicit layout. - --size only sets the render size again. Tile size needs --tile-size. Layout flags are validated before any download or render runs. - Coordinates entries carry page and atlas. The per-entry page width and height are dropped since the PNG already knows its own size. - Numbered page files left over from an earlier, larger run are removed. - pageOutputPath only looks at the file name, so a dotted directory no longer gets the page number spliced into it. - overlay_definitions.json is an input, not an output, so it moves into the tooling resources and --overlay-definitions can point elsewhere. A missing or malformed file is an error instead of a warning. - Cleanup is opt-in for a custom --work-dir (--clean) and refuses to delete a directory that holds the current directory, the output or a renderer checkout the caller placed inside it. Force re-clone gets the same current-directory guard. - Atlas and JSON write failures throw instead of printing and carrying on. - Shared flag parsing in CliArgs, with errors that name the flag. - The Mockito agent line moves from the parent pom into the generator module, the only one that mocks, so tooling no longer needs a surefire override. Tests cover the layout arithmetic and validation, flag parsing, path resolution and cleanup guards, paging boundaries (exact capacity, one over, padded rows, a shorter last page, stale pages), both concrete generators against temp directories, and the tool's parse step.
The tooling can now write the item atlas across several pages, but the library loader only knew about one file. SpritesheetLoader parses the coordinates file up front and decodes each page the first time a texture on it is asked for, keyed by the entry's atlas field. Entries without one still come from minecraft_texture_atlas.png, so the committed single-page resources load exactly as before. Spritesheet keeps its three static methods as a facade over one loader, so no caller changes. Two visible differences: a missing or malformed coordinates file or page now throws GeneratorException on first use instead of logging and handing back nulls, and getImageMap returns an unmodifiable snapshot in file order rather than the live map. Tests build two small page PNGs in a temp directory and check page pixels, that page one is never opened until needed, the no-atlas fallback, every validation error with its path, and that concurrent callers open each resource once.
Rewrites the section added in #97 for what the flags do now: --size is render-only, --tile-size sets the atlas tile, cleanup is opt-in for a custom work directory, the overlay definitions ship with the tooling, and the library reads paged atlases.
Four things in the overlay pipeline had quietly stopped working, all in the trim feature and all checked against the real 26.3 client: - Trim overlay discovery split file names on the last underscore and then looked for "<type>_trim.png". The game only ships helmet_trim.png and its three siblings, so every candidate became "trim_trim" and no trim overlay was ever packed. Each "<armor>_trim.png" now becomes a MAPPED overlay named after the file. - Trim palettes moved from textures/trims/color_palettes to textures/palettes/trim, with the key palette at textures/palettes/trim_base.png. The extractor looked only in the old place, so armor_trim colours silently dropped out of overlay_colors.json and the spritesheet stopped filtering the several hundred "<armor>_<material>_trim" renders. Both layouts are supported now, current one first. - The grey-to-index map for trims was a hard-coded constant that no longer matches the key palette: the overlays are drawn in f1, e1, d0, bc and so on, while the constant said e0, c0, a0, 80. The map is now read from the key palette itself, and a material palette with a different number of colours is skipped with a warning instead of producing an index that would overflow at render time. - Mojang stopped publishing obfuscation mappings with 26.1 and the JAR now carries real class names, but the colour extractor refused to run without a mappings file, which took the whole --overlays step, and so --all, down. The extractor reads an unmapped JAR directly, checks that the named classes really are present before trusting it, and the downloader says plainly when a version has no mappings. Verified against the 26.3 client: 4 trim overlays, 16 trim palettes, 16 dye, 39 effect and 20 potion colours, and an armor_trim map that matches trim_base.png.
A read of the whole branch diff turned up a handful of loose ends, mostly in the "fails quietly" category the branch set out to remove: - A broken trim palette layout used to be caught, printed and ignored, which let every trimmed-armour render into the atlas. It now ends the run. - --overlay-definitions is parsed at startup, like the other flags, instead of after the download and render have already run. A null colorMode or a non-string armour pattern in that file is reported as a definitions error naming the file rather than escaping as a raw Gson exception. - Unknown options and valued flags given without a value are rejected by all three entry points instead of being ignored, so a typo like --atlas-hieght cannot silently change the output. - A renderer that is present but fails now fails the run. Only a missing .NET SDK still skips the step with a warning. - --version is trimmed, matching the other flags. - Stale page removal scans the directory for numbered siblings, so a gap in the numbering no longer hides later pages. - isDefaultWorkDir compares absolute paths, so an absolute or ./ spelling of tooling/build keeps the default cleanup. - With several client JARs cached and no --version, the colour config asks for one instead of picking whichever file lists first. - The colour extraction failure message no longer tells people to fetch mappings that Mojang stopped publishing. - The generator loader decodes a page before entering the per-texture cache, so a slow decode only waits on callers of that page. - ToolingConstants holds no constants any more and is renamed ToolingSupport; the unused public palette overload is gone.
The direct-commit step pushed to master and the PR step targeted master no matter which branch the workflow was dispatched from, so running it inside an open PR either failed at the push or produced a second PR carrying the whole branch. Both now use the dispatched branch; on master nothing changes.
MinecraftRenderer moved to SixLabors ImageSharp 4 in May, and that library refuses to compile without a Six Labors license key. The pipeline run on this branch was the first to build it since then and died in a wall of NuGet output. The tool now checks for the SIXLABORS_LICENSE_KEY variable or a sixlabors.lic file before it clones or builds anything, and explains where to get a key when neither is there. The workflow passes the repository secret of the same name through to the build, which is how the renderer's own CI does it.
The render step cloned and built MinecraftRenderer, a .NET project that has needed a Six Labors ImageSharp license to compile since May. It is replaced by minecraft-library/asset-renderer, a Java library under Apache-2.0 that reads the client JAR the download step already cached and draws every inventory icon inside the tool's own JVM. - ItemIconRenderer takes the item list from the client's own registry, the one-definition-per-item files under assets/minecraft/items, and renders it through the library's atlas renderer in batches of 128, cutting each sheet into one PNG per item. An item the library cannot draw is reported by id. - Against the old atlas the 153 new names are real 26.x items and the 339 dropped names are model variants and technical blocks such as clock_27, bow_pulling_1 and potted_dandelion; the generator never referenced any of them. - --renderer-dir, --force-clone, --force-build, the .NET and git checks, the license precondition and the renderer clause of the cleanup guard go away with the old renderer. ToolingPaths is a work directory and an output directory, and the cached-version lookup moves there since two steps need it. - The workflow no longer installs .NET and gives Maven's JVM 4 GB for the renderer's context. Checked on the 26.3 client: 1,657 items in about 25 seconds at 256px, a single 4096 by 26624 atlas page, with chests, beds, signs and heads drawn as the block entities they are.
asset-renderer draws block icons the way a vanilla inventory slot shows them, slightly smaller inside the tile and with vanilla's top-to-side face shading, where the old renderer filled the tile with flatter faces. Flat item sprites are unchanged, which is why diamond_sword.png stays as it was. The six goldens that contain a block icon are recaptured from the regenerated atlas, with the two manual regeneration methods the tests ship for exactly this.
It belonged to an old side project: every step installs and runs Tessera modules that are not in this repository and it points at a CONTRIBUTING-BASELINES.md that does not exist, so it could never have run here. The golden images in this repository are recaptured locally with the regeneration methods the tests themselves ship.
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
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 this is
The follow-up to #97. That PR added configurable directories and paged atlases; this one finishes the job the review asked for, fixes the armour trim path, which had been broken since Minecraft 26.1, and replaces the external .NET renderer with an in-process Java one.
Rendering
The render step used to clone and build ptlthg's MinecraftRenderer, a .NET project. That project moved to SixLabors ImageSharp 4 in May, which refuses to compile without a Six Labors license key, so the step has been unbuildable since then. It now uses asset-renderer (Java, Apache-2.0, pinned to a commit through JitPack) running inside the tool's own JVM: no clone, no build, no .NET SDK, no license.
assets/minecraft/items/definitions in the client jar the download step already cached.clock_27,bow_pulling_1andpotted_dandelion, which nothing in the generator referenced.--renderer-dir,--force-cloneand--force-buildare gone with the old renderer, and the workflow no longer installs .NET.Tooling
ToolingPathsvalue built once per entry point from flags, then system properties, then the repository defaults, and passed to every component. The old system-property-in-static-initialiser trick is gone, along with the overlay entry point that never read the flags.AtlasLayoutcode path inAtlasGenerator.--sizeonly sets the render size again; tile size needs--tile-size. Layout flags,--sizeand--overlay-definitionsare validated before any download or render starts.pageandatlas; the per-entry page width and height are gone since the PNG knows its own size. Numbered page files left over from an earlier, larger run are removed, including past a gap in the numbering.overlay_definitions.jsonis an input, not an output. It ships with the tooling and--overlay-definitionscan point elsewhere. A missing or malformed file is an error.--work-dir(--clean) and refuses to delete a directory that holds the current directory or the output.Trim pipeline, checked against the real 26.3 client
<armor>_trim.pngnow becomes a MAPPED overlay.textures/palettes/trim/with the key attextures/palettes/trim_base.png. Both layouts are supported, and a palette layout that is present but broken fails the run rather than letting every trimmed render into the atlas.Generator
SpritesheetLoaderis page-aware and decodes each page on first use, behind the unchangedSpritesheetstatics. Entries without anatlasfield still load from the single bundled file. Loading errors throwGeneratorExceptioninstead of handing back nulls, andgetImageMapreturns an unmodifiable snapshot.Build
Verification
mvn verify: 127 tooling tests and 1,568 generator tests, editorconfig clean on all modules.armor_trimmap that matchestrim_base.png.After merging
Run the Generate Spritesheet workflow once (or dispatch it on this branch first). The committed assets date from 1.21.11, so expect a large diff: a regenerated atlas, 13 overlays instead of 9, a corrected
armor_trimcolour map, andpage/atlasfields in the coordinates.Not in this PR: the library binds one overlay per item and
item_overlay_binding.jsonnever points at the trim overlays, so trimmed armour still renders without its trim. That needs a decision about the binding format.