Skip to content

Detect tileset color keys from the mapped space glyph - #190

Merged
HexDecimal merged 3 commits into
libtcod:mainfrom
snowyukitty:fix/tileset-space-color-key
Oct 11, 2026
Merged

HexDecimal merged 3 commits into
libtcod:mainfrom
snowyukitty:fix/tileset-space-color-key

Conversation

@snowyukitty

@snowyukitty snowyukitty commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

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 by build/tests/unittest "[tileset]" and build/tests/unittest "~[!nonportable]"; the SDL-disabled build also used LIBTCOD_SDL3=OFF. Sanitizer validation used the [tileset] selection. Changed-file checks used pre-commit run --files src/libtcod/tileset.c tests/test_tileset.cpp CHANGELOG.md.

@codecov

codecov Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.26471% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 32.65%. Comparing base (aa344a4) to head (e8fe37d).

Files with missing lines Patch % Lines
src/libtcod/tileset.c 95.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@HexDecimal HexDecimal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@HexDecimal

Copy link
Copy Markdown
Collaborator

In addition, please rebase onto main to fix CI issues (Vcpkg submodule update).

@snowyukitty

Copy link
Copy Markdown
Contributor Author

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 HexDecimal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final suggested changes.

Comment thread src/libtcod/tileset.c Resolved
Comment thread CHANGELOG.md Resolved
Comment thread src/libtcod/tileset.c Resolved
@snowyukitty

Copy link
Copy Markdown
Contributor Author

Applied the final suggestions in e8fe37d: tile zero is checked first, with the first assigned space used only as a fallback. Simplified the space-index helper as suggested; the sampling helper retains the bounds check. Updated the changelog wording and regression tests for tile-zero priority and duplicate space mappings.

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 HexDecimal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@HexDecimal
HexDecimal merged commit 58f32d4 into libtcod:main Oct 11, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants