Repository navigation
Detect tileset color keys from the mapped space glyph - #190
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #190 +/- ##
==========================================
+ Coverage 31.54% 32.65% +1.11%
==========================================
Files 117 117
Lines 13084 13216 +132
Branches 2097 2106 +9
==========================================
+ Hits 4127 4316 +189
+ Misses 8957 8900 -57 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
HexDecimal
left a comment
There was a problem hiding this comment.
This causes a regression if it is the space character which contains a glyph. For this to work as a fallback both the space and null tiles must be tested to see if they're opaque for their key color.
Consider splitting these up into static functions: fetching the tile index of the space character, checking the tile index for a solid color.
Make sure to add this change to the changelog as a fix.
|
In addition, please rebase onto |
|
Updated the fallback to check the mapped space first, then tile zero; neither is used as the key unless its pixels form a solid RGBA color. Split the space mapping and solid-color checks into static helpers and added the changelog entry. The branch also includes current main's Vcpkg submodule update. The updated fallback regressions fail against the previous PR implementation and pass with the repair. The portable, SDL-disabled and tileset sanitizer checks also pass locally. |
HexDecimal
left a comment
There was a problem hiding this comment.
Final suggested changes.
|
Applied the final suggestions in The updated regressions fail against the previous PR implementation and pass with this change. Locally, 78 portable cases, 75 SDL-disabled cases, 13 tileset cases under ASan/UBSan, and the changed-file pre-commit hooks pass. |
HexDecimal
left a comment
There was a problem hiding this comment.
This all looks good! Especially the new tests.
Codecov shows a minor case for a charmap without spaces is not covered, but I'm unbothered by that.
If you need a new release of libtcod with this merged then now is a good time to tell me.
Detect a tilesheet color key from tile zero when its pixels have a solid RGBA color, preserving existing font behavior. If tile zero contains a glyph, use the first assigned U+0020 tile only when its pixels also have a solid RGBA color. If neither candidate is solid, preserve the colors. Null character maps use implicit U+0020 when present; the sampling helper checks tile-index bounds.
Static helpers separate space mapping from solid-color detection. Sampling keeps the same tile origin and image stride as upload, including partial image columns. Existing grayscale/alpha normalization, public APIs and ABI are unchanged. The fix is recorded in the changelog.
Regression coverage includes mapped and implicit spaces, a nonuniform space with a solid tile zero, both candidates containing glyphs, preference for a solid first tile over a different solid space, first-space selection with duplicate mappings, limited and out-of-range mappings, partial columns, grayscale normalization and software-surface rendering.
Refs #157.
Validation in disposable unprivileged Linux containers, with networking disabled during tests: 78 portable cases passed; 75 cases passed with SDL disabled; all 13 tileset cases passed under ASan/UBSan with leak detection. The updated priority and duplicate-space regressions fail against the previous PR implementation (two cases, four assertions) and pass with this repair. Repository pre-commit hooks passed for the three changed files. GUI/nonportable tests and native Windows/macOS runs were not performed. No new hosted-CI pass is claimed.
From the repository root, CMake builds used
LIBTCOD_TESTS=ON, followed bybuild/tests/unittest "[tileset]"andbuild/tests/unittest "~[!nonportable]"; the SDL-disabled build also usedLIBTCOD_SDL3=OFF. Sanitizer validation used the[tileset]selection. Changed-file checks usedpre-commit run --files src/libtcod/tileset.c tests/test_tileset.cpp CHANGELOG.md.