Conversation
- Replace stbi_write_png_to_mem with stbi_write_png_to_func in spratunpack_command.cpp to avoid "undeclared identifier" errors in some stb_image_write.h versions. - Reorder CMakeLists.txt to detect dependencies (libarchive, gettext, zopflipng) before defining targets. This fixes "archive.h not found" errors on macOS where headers are in non-standard brew paths. - Remove redundant source file inclusion in executable targets by linking to spratcore library.
- Replace stbi_write_png_to_mem with stbi_write_png_to_func in spratunpack_command.cpp for better portability and fix build errors. - Remove redundant extern "C" around stb_image_write.h include. - Reorder CMakeLists.txt to ensure dependencies (libarchive, gettext, zopflipng) are detected before target definition, fixing macOS build failures. - Optimize build by removing redundant source inclusion in executable targets. - Bump version to v0.2.7 in VERSION file to resolve immutable release error in GitHub Actions.
- Exclude temporary documentation snapshots in .gitignore - Add unit tests for compare_natural() function covering: natural number sorting, mixed alphanumeric, edge cases - All 14 tests passing (2 unit + 12 integration)
- Add -DBUILD_TESTING=ON to CMake configure steps in both jobs - Update ctest invocations to emit JUnit XML test results - Add artifact upload steps for test results (7-day retention) - Add artifact retention policy (30 days for packages) - Add dependency caching for Linux apt packages and macOS Homebrew - Update Linux apt-get to use --no-upgrade flag for cached packages
- Implement --deduplicate flag with FNV-1a content hashing - Implement --gpu-compress dxt1|dxt5 for GPU-native texture formats - Implement --dilate N for color bleeding to prevent GPU filtering artifacts - Add libsquish library integration (optional dependency) - Bump cache format version (2 → 3) - Update CMakeLists.txt with squish detection
Release v0.3.0 includes the new --deduplicate feature with support for: - Exact deduplication (byte-for-byte identical images) - Perceptual deduplication (visually similar images using dHash) - Full cache management and downstream compatibility Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Enhance spratlayout command to support three deduplication modes instead of a boolean flag: - none: No deduplication (default) - exact: Hash-detect byte-for-byte identical sprites (FNV-1a) - perceptual: Detect visually similar sprites (dHash algorithm) Changes: - Updated --deduplicate argument to accept mode value - Modified cache signature to include full mode string - Added validation for mode values with helpful error messages - Updated function signatures to pass mode string instead of boolean This enables the sprat-gui UI to properly use the deduplication modes as expected by its settings UI. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The spratcore static library includes command implementations that use libarchive functions, but wasn't linking against libarchive. This caused unresolved symbol linker errors on Windows when executables (spratlayout, spratpack, spratunpack) tried to link against spratcore. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Hoist vector/map allocations out of hot loops, replace ostringstream with string concatenation and snprintf, pre-group sprites by atlas index, rewrite prune_free_rects with mark-and-sweep, use region-only copies in dilate, deduplicate pixel extraction in unpack, and replace queue with reusable vector stack in flood fill.
- Add support for negated filter attributes in convert/layout commands - Improve --threads parameter documentation with clearer behavior and defaults - Remove max_combinations from config keys documentation - Update transform definitions
…parsing When input is a list file, the working_folder (the list file path itself) was passed directly as the root for relative path computation, producing incorrect sprite paths like "../frames/b.png". Pass parent_path() instead at both build_layout_output_text call sites. The layout parser also didn't handle the "root" line emitted by spratlayout, causing spratpack to fail with "Unknown line: root ...". Add root field to Layout struct, parse it in layout_parser, and resolve relative sprite/alias paths against it in spratpack.
…ount64 __builtin_popcountll is a GCC/Clang intrinsic not available on MSVC. Add a popcount64() wrapper that uses __popcnt64 via <intrin.h> on Windows and __builtin_popcountll elsewhere.
…placeholders
- Replace specialized conditional blocks ([atlas_path], [rotated],
[if_animations], etc.) with a general [if ATTR="VALUE"]...[/if]
syntax usable at any nesting level
- Restructure JSON transform: move sprites to a flat top-level array;
group spatial data into nested rect, pivot, and trim objects
- Add {{name_css}} placeholder for CSS-safe sprite class names; CSS
transform now uses .sprite-{{name_css}} instead of .sprite-{{index}}
- Add {{sprite_names}}/{{sprite_names_json}}/{{sprite_names_csv}}
placeholders for animation frame display names
- Add --transforms-dir and --default-profiles-config diagnostic flags
- Add label= field to profile definitions; adjust desktop/legacy
default padding; enable rotate in space profile
- Fix parallel CMake build race when copying transforms/ by
consolidating into a single sprat_copy_transforms custom target
- Improve image cache: validate dimensions on save, evict oldest
entries when over the size limit instead of refusing to save
- Fix sprite path resolution: index all path suffixes so absolute
or temp-prefixed paths match layout entries correctly
- Capitalize built-in transform names (JSON, CSV, XML, CSS)
- Update man page date and document new features
There was a problem hiding this comment.
Code Review
This pull request updates the sprat-cli suite to version 0.5.1, introducing sprite deduplication, artifact reduction via dilation, and GPU-native texture compression. It refactors the transform system for better metadata control and optimizes core packing algorithms. Feedback focuses on reducing memory overhead during dilation, adopting modern C++20 idioms, ensuring arithmetic safety against overflows, and improving maintainability by replacing magic numbers and manual hash implementations with named constants and shared utilities.
|
|
||
| // Two-buffer approach: read_buf holds the previous pass state, atlas is the write target. | ||
| // After each pass we copy only the affected sprite region back instead of the full atlas. | ||
| std::vector<unsigned char> read_buf = atlas; |
There was a problem hiding this comment.
std::vector<unsigned char> read_buf = atlas; creates a full copy of the atlas buffer. For large atlases (e.g., 16k x 16k), this can consume a significant amount of memory (1GB for RGBA). Consider using a smaller temporary buffer only for the region being dilated or optimizing the two-buffer approach to avoid a full atlas copy.
| #ifdef _MSC_VER | ||
| static inline int popcount64(unsigned long long x) { return (int)__popcnt64(x); } | ||
| #else | ||
| static inline int popcount64(unsigned long long x) { return __builtin_popcountll(x); } | ||
| #endif |
There was a problem hiding this comment.
Since the project is using C++20 features (like std::ranges::sort), it is better to use std::popcount from the <bit> header instead of manual compiler-specific intrinsics. This improves portability and readability.
| #ifdef _MSC_VER | |
| static inline int popcount64(unsigned long long x) { return (int)__popcnt64(x); } | |
| #else | |
| static inline int popcount64(unsigned long long x) { return __builtin_popcountll(x); } | |
| #endif | |
| static inline int popcount64(unsigned long long x) { return std::popcount(x); } |
| #include <unordered_map> | ||
| #include <unordered_set> | ||
| #include <optional> | ||
| #include <numeric> | ||
| #include <cstring> |
There was a problem hiding this comment.
Please include the <bit> header to support the use of std::popcount.
| #include <unordered_map> | |
| #include <unordered_set> | |
| #include <optional> | |
| #include <numeric> | |
| #include <cstring> | |
| #include <unordered_map> | |
| #include <unordered_set> | |
| #include <optional> | |
| #include <numeric> | |
| #include <cstring> | |
| #include <bit> |
| size_t write = 0; | ||
| for (size_t ri = 0; ri < free_rects.size(); ++ri) { | ||
| if (free_rects[ri].w > 0 && free_rects[ri].h > 0) { | ||
| if (write != ri) { | ||
| free_rects[write] = free_rects[ri]; | ||
| } | ||
| ++write; | ||
| } | ||
| } | ||
| prune_free_rects(free_rects); | ||
| free_rects.resize(write); |
There was a problem hiding this comment.
This manual compaction loop can be replaced with std::erase_if (available since C++20), which is more idiomatic and concise.
| size_t write = 0; | |
| for (size_t ri = 0; ri < free_rects.size(); ++ri) { | |
| if (free_rects[ri].w > 0 && free_rects[ri].h > 0) { | |
| if (write != ri) { | |
| free_rects[write] = free_rects[ri]; | |
| } | |
| ++write; | |
| } | |
| } | |
| prune_free_rects(free_rects); | |
| free_rects.resize(write); | |
| std::erase_if(free_rects, [](const Rect& r) { return r.w <= 0 || r.h <= 0; }); |
| uint64_t fnv = 14695981039346656037ULL; | ||
| for (size_t bi = 0; bi < nbytes; ++bi) { | ||
| fnv ^= px[bi]; | ||
| fnv *= 1099511628211ULL; | ||
| } | ||
| entry_content_hash = fnv; |
There was a problem hiding this comment.
The PR adds src/core/fnv1a.h which provides a reusable fnv1a_hash function. It should be used here instead of re-implementing the algorithm manually with magic numbers.
| uint64_t fnv = 14695981039346656037ULL; | |
| for (size_t bi = 0; bi < nbytes; ++bi) { | |
| fnv ^= px[bi]; | |
| fnv *= 1099511628211ULL; | |
| } | |
| entry_content_hash = fnv; | |
| entry_content_hash = sprat::core::fnv1a_hash(px, nbytes); |
| } | ||
| sprites = std::move(deduped); | ||
| } else if (deduplicateMode == "perceptual") { | ||
| // O(N²) pairwise + union-find dedup |
There was a problem hiding this comment.
The perceptual deduplication uses an
| // Union-find | ||
| std::vector<size_t> parent(N); | ||
| std::iota(parent.begin(), parent.end(), 0); | ||
| std::function<size_t(size_t)> find = [&](size_t x) -> size_t { |
There was a problem hiding this comment.
| size_t compressed_bytes = (format == "dxt1" || format == "DXT1") | ||
| ? (width * height) / 2 | ||
| : width * height; |
There was a problem hiding this comment.
The calculation of compressed_bytes uses int for width and height. While unlikely given typical atlas limits, width * height could overflow a 32-bit signed integer. It is safer to cast to size_t before multiplication.
| size_t compressed_bytes = (format == "dxt1" || format == "DXT1") | |
| ? (width * height) / 2 | |
| : width * height; | |
| size_t compressed_bytes = (format == "dxt1" || format == "DXT1") | |
| ? (static_cast<size_t>(width) * height) / 2 | |
| : static_cast<size_t>(width) * height; |
| DdsHeader header = {}; | ||
| header.magic = 0x20534444; // "DDS " | ||
| header.size = 124; | ||
| header.flags = 0x0001 | 0x0002 | 0x0004 | 0x1000; // CAPS | HEIGHT | WIDTH | LINEARSIZE |
On multi-config generators (MSVC, Xcode) CMake places binaries in a per-config subdirectory (e.g. build/Release/), but sprat_copy_transforms was copying transforms/ to the bare CMAKE_RUNTIME_OUTPUT_DIRECTORY or CMAKE_CURRENT_BINARY_DIR without the config suffix. At runtime find_transforms_dir() looks for transforms/ next to the executable and could not find them, causing the convert and output_pattern CI tests to fail on Windows with "Missing transform in list" and "Failed to open transform file". Detect multi-config generators via GENERATOR_IS_MULTI_CONFIG and append /$<CONFIG> to the destination path so transforms land in the same directory as the binaries, matching the behaviour of the old per-target $<TARGET_FILE_DIR:...> commands.
Search order is now: executable directory → user directory →
global system directory. The CWD candidate for profiles config
is removed. The bare relative "transforms" fallback is replaced
with the global install path.
Platform user directories are now correct:
- Linux: XDG_CONFIG_HOME (profiles) and XDG_DATA_HOME (transforms),
defaulting to ~/.config and ~/.local/share respectively
- macOS: ~/Library/Application Support/sprat/ for both
(was ~/Library/Preferences/ which is for .plist files only)
- Windows: %APPDATA%\sprat\ (unchanged)
spratlayout no longer sorts sprites by filename when reading from a
folder. The default is now none for all input types, allowing the
packer to reorder sprites freely for better packing efficiency.
Use --sort name to restore alphabetical ordering.
Man page updates:
- spratlayout synopsis now shows folder|file|- to document all
three input modes (directory, list file, TAR from stdin)
- Input modes section explains each mode including the - stdin TAR
path and its use with spratunpack output
- spratunpack atlas input and output documented as separate sections
- Profile config search order updated: beside executable, then user
directory (with correct per-platform paths), then global
- Removed --max-combinations (feature was removed)
- --sort description clarified; default updated to none
- Transform file search path documented in spratconvert section
- Examples expanded: list file input, full pipeline with conversion,
stdin TAR round-trip (spratunpack | spratlayout -), tar cf - pipe
- Parallelize sprite loading and optimize compact packing search.
Implement dynamic scheduling (atomic work index) for packing threads.
- Merge guided and shelf packing passes and add early-exit pruning.
- Enable recursive directory scanning by default in spratlayout.
- Add support for .spratlayoutignore and 'exclude' directive in list files.
- Introduce 'root' directive for animations, markers, and list files.
- Document changes in README.md and man pages.
- Add regression tests for recursive scanning and exclusions.
Replace the animation alias h-flip/v-flip token pair with a single
flip keyword followed by a value: "h", "v", or "vh". The layout file
syntax changes from:
alias "run" h-flip v-flip
to:
alias "run" flip vh
The template variable is similarly unified: the two boolean vars
h_flip/v_flip are replaced by a single string var flip. Built-in
transforms (JSON, CSV, XML) are updated accordingly.
Add --sort stable[:<metric>] to spratlayout. This mode sorts sprites
deterministically by a geometric metric (area, maxside, height, width,
or perimeter; default: area) with natural path order as the tiebreaker.
Sources are always sorted by path first, fixing filesystem
non-determinism. The stable order is preserved through the packing loop
without trying alternative sort modes, so the layout and file assignment
are reproducible across runs. Adding or removing sprites only locally
disturbs the atlas rather than scrambling the whole ordering.
Update spratprofiles.cfg with more sensible defaults: desktop gains
explicit max dimensions (8192×8192), legacy switches to optimize=gpu for
square POT textures preferred by old hardware and bumps its limit to
2048, css switches from fast to compact mode for better packing quality,
and redundant scale=1 entries are removed throughout.
Add --stdin-list option to read image paths from stdin Introduces a new `--stdin-list` flag that allows piping a list of image paths (one per line) directly to spratlayout without requiring a folder argument or a list file on disk. Refactors the list-parsing logic into a shared `parse_list_stream` lambda, eliminating duplication between the ListFile and new StdinList input paths. Both now support the same `exclude` and `root` directives. On Windows, stdin is explicitly set to text mode when using --stdin-list.
Every frame is placed in a uniform cell (max frame width × max frame height), arranged left-to-right, top-to-bottom, so any sprite can be looked up by (col, row) without parsing the layout file. Column count targets a square atlas and widens automatically when --max-width or --max-height are set. Multipack splits the sequence into equal-capacity grid atlases. Tight-bounds trimming is suppressed so cell dimensions stay uniform even with padding > 0. New profile "grid" added to spratprofiles.cfg; man page updated.
- Added new template placeholders: {{unity_y}}, {{pivot_x_norm}}, {{pivot_y_norm}}, {{pivot_y_norm_raw}},
{{name_hash}}, and {{name_hash_hex}}.
- Created unity.json and unity.meta transforms for seamless Unity engine integration.
- Updated Phaser and Godot transforms to include normalized pivot data.
- Updated README.md and man pages with documentation for the new fields.
- Added automated test/unity_test.sh to the CMake test suite.
Renames the --output flag to --atlas in both spratconvert and spratpack commands to better reflect its purpose as an atlas path pattern. - Adds -a as the new preferred short flag. - Maintains backward compatibility for --output and -o. - Updates man pages, README, and help messages to reflect new naming. - Updates regression tests to verify both new and legacy flags.
Remove the custom section-based DSL ({{placeholders}},
[meta]/[header]/
[sprites] tags) and replace it with google/jsonnet v0.20.0 as the
transform engine. Transforms are now .jsonnet files that receive the
full layout data as std.extVar("sprat") and return a JSON object with
a content or files field.
- Add google/jsonnet via CMake FetchContent, pinned to SHA
f45e01d (v0.20.0); link libjsonnet++ and libjsonnet_static for
correct static linking on all platforms
- Remove ~1400 lines of DSL parser/renderer from
spratconvert_command.cpp;
add build_sprat_json(), evaluate_transform(), and
parse_transform_result()
- Add sprat.libsonnet with format_double() (works around a %g bug in
Jsonnet v0.20) and consecutive_runs() for non-contiguous frameTags
- Replace all 11 .transform files with 14 .jsonnet files: json, csv,
xml, css, aseprite, libgdx, godot, phaser-hash, phaser-array,
phaser-anims, plist, unity.json, unity.meta, unity.anim
- Multi-file transforms (unity.anim) return a files array instead of
content and write one file per animation under --output-dir
- Update tests to Jsonnet format and add coverage for non-contiguous
animations (aseprite frameTags), multipack atlas_index, and empty
animations file
- Update README: replace DSL placeholder reference with Jsonnet data
model and built-in transform table
Grid mode was silently accepting sprites of different sizes and placing them using a cell stride derived from the largest sprite, while each sprite's w/h in the layout remained its own original dimensions. This made the layout inconsistent: coordinates implied one cell size, metadata implied another. Grid mode now validates that all sprites have the same dimensions before packing. If any sprite differs, a diagnostic names the offending file and the expected size, and the layout is rejected. The max-dimension loop is also replaced with a direct cell_w = sprite_w + padding since the sizes are guaranteed equal after validation. The fix applies to both the single-atlas path (pack_grid) and the multipack path (pack_atlases).
No description provided.