refactor(core/rendering): deduplicar Text::write* i nomenar magic numbers (DUP-03 + HC-01)

Text tenia cinc mètodes (write, writeColored, writeMonoDigits, length,
lengthMonoDigits) i dos getters (glyphWidth, getGlyphClip) que repetien
el mateix patró de resolució de codepoint: find(cp), fallback a '?' si
no hi és. Extret a un helper privat resolveGlyph(cp) que centralitza
aquesta lògica; totes les funcions passen a usar-lo.

write() ara delega directament a writeColored(...) passant el color per
defecte, eliminant ~19 línies del bucle UTF-8 duplicades.

Els literals 1 (índex de "tinta" al PNG) i 15 (color per defecte a la
paleta) passen a ser Text::INK_COLOR_INDEX i Text::DEFAULT_TEXT_COLOR
(HC-01).

Build + smoke 5s xvfb OK (exit 124).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
JailDesignerandClaude Opus 4.7 committed 2026-04-20 09:32:37 +02:00
1 parent 5f46fb22a4
commit a63deac6a3
3 files changed
+47 -53

No files matched your search

+5 -4
View File
@@ -21,7 +21,7 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
| COUP-01 | Inversión de dependencia `core/rendering → game/*` | DONE | | COUP-01 | Inversión de dependencia `core/rendering → game/*` | DONE |
| DUP-02 | `Surface::render` / `renderWithColorReplace` duplicados | DONE | | DUP-02 | `Surface::render` / `renderWithColorReplace` duplicados | DONE |
| CONV-03 | Shaders MSL como raw strings en `.cpp` + helpers duplicados | DONE | | CONV-03 | Shaders MSL como raw strings en `.cpp` + helpers duplicados | DONE |
| DUP-03 | Familia `Text::write*` con bucle UTF-8 repetido 5 veces | PENDING | | DUP-03 | Familia `Text::write*` con bucle UTF-8 repetido 5 veces | DONE |
| COUP-02 | `Screen` instancia `SDL3GPUShader` directamente + `Surface` → `Screen::get()` | PENDING | | COUP-02 | `Screen` instancia `SDL3GPUShader` directamente + `Surface` → `Screen::get()` | PENDING |
--- ---
@@ -32,7 +32,7 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
|----|---------------|----------|-----------|--------|-------| |----|---------------|----------|-----------|--------|-------|
| DUP-01 | `gif.cpp:238-276` | 3 ramas del switch extension saltan bloque+subloops idénticos | Importante | PENDING | helper `skipExtensionBlocks` | | DUP-01 | `gif.cpp:238-276` | 3 ramas del switch extension saltan bloque+subloops idénticos | Importante | PENDING | helper `skipExtensionBlocks` |
| DUP-02 | `surface.cpp:80-137` | `render` vs `renderWithColorReplace` ~90% idénticos | Crítico | DONE | `render()` delega amb source==target==transparent | | DUP-02 | `surface.cpp:80-137` | `render` vs `renderWithColorReplace` ~90% idénticos | Crítico | DONE | `render()` delega amb source==target==transparent |
| DUP-03 | `text.cpp:145-289` | bucle UTF-8 repetido en write/writeColored/writeMonoDigits/length/lengthMonoDigits | Importante | PENDING | iterador de glifos | | DUP-03 | `text.cpp:145-289` | bucle UTF-8 repetido en write/writeColored/writeMonoDigits/length/lengthMonoDigits | Importante | DONE | `resolveGlyph` + `write` delega a `writeColored` |
| DUP-04 | `sdl3gpu_shader.cpp:165-201` + `crtpi_pipeline.cpp:165-202` | `createShaderMSL/SPIRV` duplicados | Importante | DONE | `sdl3gpu/shader_factory.hpp` | | DUP-04 | `sdl3gpu_shader.cpp:165-201` + `crtpi_pipeline.cpp:165-202` | `createShaderMSL/SPIRV` duplicados | Importante | DONE | `sdl3gpu/shader_factory.hpp` |
| DUP-05 | `sdl3gpu_shader.cpp:18-35` + `crtpi_pipeline.cpp:17-34` | vertex shader MSL duplicado | Menor | DONE | `sdl3gpu/msl/postfx_vert.msl.h` compartit | | DUP-05 | `sdl3gpu_shader.cpp:18-35` + `crtpi_pipeline.cpp:17-34` | vertex shader MSL duplicado | Menor | DONE | `sdl3gpu/msl/postfx_vert.msl.h` compartit |
| DUP-06 | `surface.cpp:14-43` | constructor Surface abre y parsea GIF 2 veces | Importante | PENDING | `loadGif` devuelve pixels+palette | | DUP-06 | `surface.cpp:14-43` | constructor Surface abre y parsea GIF 2 veces | Importante | PENDING | `loadGif` devuelve pixels+palette |
@@ -42,7 +42,7 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas | | ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|----|---------------|----------|-----------|--------|-------| |----|---------------|----------|-----------|--------|-------|
| HC-01 | `text.cpp:159` | `render(1, 15)` — `1` (tinta) y `15` (color por defecto) literales | Importante | PENDING | `INK_INDEX` / `DEFAULT_COLOR` | | HC-01 | `text.cpp:159` | `render(1, 15)` — `1` (tinta) y `15` (color por defecto) literales | Importante | DONE | `Text::INK_COLOR_INDEX` i `DEFAULT_TEXT_COLOR` |
| HC-02 | `surface.cpp:29`, `screen.cpp:56,218,244` | `0xFF000000U`/`0xFF` alfa repetido | Menor | PENDING | `OPAQUE_ALPHA` | | HC-02 | `surface.cpp:29`, `screen.cpp:56,218,244` | `0xFF000000U`/`0xFF` alfa repetido | Menor | PENDING | `OPAQUE_ALPHA` |
| HC-03 | `gif.cpp:71,95,…` | LZW `<11`/`<12`/`<2` y máscaras `0x80/0x70/0x07…` sin nombre | Menor | PENDING | constantes con nombre | | HC-03 | `gif.cpp:71,95,…` | LZW `<11`/`<12`/`<2` y máscaras `0x80/0x70/0x07…` sin nombre | Menor | PENDING | constantes con nombre |
| HC-04 | `notifier.hpp:41-46`, `render_info.hpp:38-40` | HUD constants hardcoded en clase, no en `defaults`/YAML | Importante | PENDING | `Defaults::Hud::…` o config.yaml | | HC-04 | `notifier.hpp:41-46`, `render_info.hpp:38-40` | HUD constants hardcoded en clase, no en `defaults`/YAML | Importante | PENDING | `Defaults::Hud::…` o config.yaml |
@@ -123,4 +123,5 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
- `9434e4e` — COUP-01a: `Notifier` desacoblat de `game/ui/notification_presets` - `9434e4e` — COUP-01a: `Notifier` desacoblat de `game/ui/notification_presets`
- `9395520` — COUP-01b: `RenderInfo` desacoblat de `game/options` + `game/ui/ui_palette` - `9395520` — COUP-01b: `RenderInfo` desacoblat de `game/options` + `game/ui/ui_palette`
- `d479475` — DUP-02: `Surface::render` delega a `renderWithColorReplace` - `d479475` — DUP-02: `Surface::render` delega a `renderWithColorReplace`
- `<pendent>` — CONV-03 + DUP-04 + DUP-05 + PATCH-07: shaders MSL i helpers extrets a `sdl3gpu/msl/` i `sdl3gpu/shader_factory.hpp` - `5f46fb2` — CONV-03 + DUP-04 + DUP-05 + PATCH-07: shaders MSL i helpers extrets a `sdl3gpu/msl/` i `sdl3gpu/shader_factory.hpp`
- `<pendent>` — DUP-03 + HC-01: `Text` amb `resolveGlyph` i constants `INK_COLOR_INDEX` / `DEFAULT_TEXT_COLOR`
+31 -49
View File
@@ -140,27 +140,10 @@ Text::Text(const std::shared_ptr<Surface>& surface, const std::shared_ptr<File>&
offset_(text_file->offset) { offset_(text_file->offset) {
} }
// Dibuixa la cadena amb la tinta per defecte (codi 15 a la paleta del png). // Dibuixa la cadena amb la tinta per defecte. Els glifs amb codepoint no
// Els glifs amb codepoint no registrat es substitueixen pel glif '?'. // registrat es substitueixen pel glif '?'.
void Text::write(int x, int y, const std::string& text, int kerning, int length) { void Text::write(int x, int y, const std::string& text, int kerning, int length) {
int shift = 0; writeColored(x, y, text, DEFAULT_TEXT_COLOR, kerning, length);
int glyphs_done = 0;
std::size_t pos = 0;
sprite_->setY(static_cast<float>(y));
while (pos < text.size()) {
if (length != -1 && glyphs_done >= length) { break; }
std::uint32_t cp = nextCodepoint(text, pos);
auto it = offset_.find(cp);
if (it == offset_.end()) { it = offset_.find('?'); }
if (it != offset_.end()) {
sprite_->setClip(static_cast<float>(it->second.x), static_cast<float>(it->second.y), static_cast<float>(box_width_), static_cast<float>(box_height_));
sprite_->setX(static_cast<float>(x + shift));
sprite_->render(1, 15);
shift += it->second.w + kerning;
}
++glyphs_done;
}
} }
void Text::writeColored(int x, int y, const std::string& text, Uint8 color, int kerning, int length) { void Text::writeColored(int x, int y, const std::string& text, Uint8 color, int kerning, int length) {
@@ -172,13 +155,11 @@ void Text::writeColored(int x, int y, const std::string& text, Uint8 color, int
while (pos < text.size()) { while (pos < text.size()) {
if (length != -1 && glyphs_done >= length) { break; } if (length != -1 && glyphs_done >= length) { break; }
std::uint32_t cp = nextCodepoint(text, pos); std::uint32_t cp = nextCodepoint(text, pos);
auto it = offset_.find(cp); if (const Offset* off = resolveGlyph(cp); off != nullptr) {
if (it == offset_.end()) { it = offset_.find('?'); } sprite_->setClip(static_cast<float>(off->x), static_cast<float>(off->y), static_cast<float>(box_width_), static_cast<float>(box_height_));
if (it != offset_.end()) {
sprite_->setClip(static_cast<float>(it->second.x), static_cast<float>(it->second.y), static_cast<float>(box_width_), static_cast<float>(box_height_));
sprite_->setX(static_cast<float>(x + shift)); sprite_->setX(static_cast<float>(x + shift));
sprite_->render(1, color); sprite_->render(INK_COLOR_INDEX, color);
shift += it->second.w + kerning; shift += off->w + kerning;
} }
++glyphs_done; ++glyphs_done;
} }
@@ -235,21 +216,20 @@ void Text::writeMonoDigits(int x, int y, const std::string& text, Uint8 color, i
sprite_->setY(static_cast<float>(y)); sprite_->setY(static_cast<float>(y));
while (pos < text.size()) { while (pos < text.size()) {
std::uint32_t cp = nextCodepoint(text, pos); std::uint32_t cp = nextCodepoint(text, pos);
auto it = offset_.find(cp); const Offset* off = resolveGlyph(cp);
if (it == offset_.end()) { it = offset_.find('?'); } if (off == nullptr) { continue; }
if (it == offset_.end()) { continue; }
const bool IS_DIGIT = (cp >= '0' && cp <= '9'); const bool IS_DIGIT = (cp >= '0' && cp <= '9');
const int GLYPH_W = it->second.w; const int GLYPH_W = off->w;
const int CELL_W = IS_DIGIT ? digit_cell_w : GLYPH_W; const int CELL_W = IS_DIGIT ? digit_cell_w : GLYPH_W;
const int GLYPH_X = IS_DIGIT ? x + shift + ((CELL_W - GLYPH_W) / 2) : x + shift; const int GLYPH_X = IS_DIGIT ? x + shift + ((CELL_W - GLYPH_W) / 2) : x + shift;
sprite_->setClip(static_cast<float>(it->second.x), sprite_->setClip(static_cast<float>(off->x),
static_cast<float>(it->second.y), static_cast<float>(off->y),
static_cast<float>(box_width_), static_cast<float>(box_width_),
static_cast<float>(box_height_)); static_cast<float>(box_height_));
sprite_->setX(static_cast<float>(GLYPH_X)); sprite_->setX(static_cast<float>(GLYPH_X));
sprite_->render(1, color); sprite_->render(INK_COLOR_INDEX, color);
shift += CELL_W + kerning; shift += CELL_W + kerning;
} }
@@ -261,12 +241,11 @@ auto Text::lengthMonoDigits(const std::string& text, int digit_cell_w, int kerni
while (pos < text.size()) { while (pos < text.size()) {
std::uint32_t cp = nextCodepoint(text, pos); std::uint32_t cp = nextCodepoint(text, pos);
auto it = offset_.find(cp); const Offset* off = resolveGlyph(cp);
if (it == offset_.end()) { it = offset_.find('?'); } if (off == nullptr) { continue; }
if (it == offset_.end()) { continue; }
const bool IS_DIGIT = (cp >= '0' && cp <= '9'); const bool IS_DIGIT = (cp >= '0' && cp <= '9');
shift += (IS_DIGIT ? digit_cell_w : it->second.w) + kerning; shift += (IS_DIGIT ? digit_cell_w : off->w) + kerning;
} }
return shift > 0 ? shift - kerning : 0; return shift > 0 ? shift - kerning : 0;
@@ -278,10 +257,8 @@ auto Text::length(const std::string& text, int kerning) const -> int {
while (pos < text.size()) { while (pos < text.size()) {
std::uint32_t cp = nextCodepoint(text, pos); std::uint32_t cp = nextCodepoint(text, pos);
auto it = offset_.find(cp); if (const Offset* off = resolveGlyph(cp); off != nullptr) {
if (it == offset_.end()) { it = offset_.find('?'); } shift += off->w + kerning;
if (it != offset_.end()) {
shift += it->second.w + kerning;
} }
} }
@@ -289,18 +266,23 @@ auto Text::length(const std::string& text, int kerning) const -> int {
} }
auto Text::glyphWidth(std::uint32_t codepoint, int kerning) const -> int { auto Text::glyphWidth(std::uint32_t codepoint, int kerning) const -> int {
auto it = offset_.find(codepoint); if (const Offset* off = resolveGlyph(codepoint); off != nullptr) {
if (it == offset_.end()) { it = offset_.find('?'); } return off->w + kerning;
if (it != offset_.end()) { return it->second.w + kerning; } }
return 0; return 0;
} }
auto Text::getGlyphClip(std::uint32_t codepoint) const -> SDL_FRect { auto Text::getGlyphClip(std::uint32_t codepoint) const -> SDL_FRect {
auto it = offset_.find(codepoint); const Offset* off = resolveGlyph(codepoint);
if (it == offset_.end()) { it = offset_.find('?'); } if (off == nullptr) { return {.x = 0.0F, .y = 0.0F, .w = 0.0F, .h = 0.0F}; }
if (it == offset_.end()) { return {.x = 0.0F, .y = 0.0F, .w = 0.0F, .h = 0.0F}; } return {.x = static_cast<float>(off->x),
return {.x = static_cast<float>(it->second.x), .y = static_cast<float>(off->y),
.y = static_cast<float>(it->second.y),
.w = static_cast<float>(box_width_), .w = static_cast<float>(box_width_),
.h = static_cast<float>(box_height_)}; .h = static_cast<float>(box_height_)};
} }
auto Text::resolveGlyph(std::uint32_t cp) const -> const Offset* {
auto it = offset_.find(cp);
if (it == offset_.end()) { it = offset_.find('?'); }
return (it != offset_.end()) ? &it->second : nullptr;
}
+11
View File
@@ -36,6 +36,13 @@ class Text {
static constexpr Uint8 CENTER_FLAG = 4; static constexpr Uint8 CENTER_FLAG = 4;
static constexpr Uint8 STROKE_FLAG = 8; static constexpr Uint8 STROKE_FLAG = 8;
// Índex del píxel "tinta" al PNG de la font. Els glifs són monocroms
// sobre transparent i tots els píxels opacs usen aquest índex; la Surface
// el recolora al vol per a emetre el color destí.
static constexpr Uint8 INK_COLOR_INDEX = 1;
// Color per defecte a la paleta quan s'usa write() sense especificar-lo.
static constexpr Uint8 DEFAULT_TEXT_COLOR = 15;
Text(const std::shared_ptr<Surface>& surface, const std::string& text_file); Text(const std::shared_ptr<Surface>& surface, const std::string& text_file);
Text(const std::shared_ptr<Surface>& surface, const std::shared_ptr<File>& text_file); Text(const std::shared_ptr<Surface>& surface, const std::shared_ptr<File>& text_file);
~Text() = default; ~Text() = default;
@@ -68,6 +75,10 @@ class Text {
static auto nextCodepoint(const std::string& s, std::size_t& pos) -> std::uint32_t; static auto nextCodepoint(const std::string& s, std::size_t& pos) -> std::uint32_t;
private: private:
// Busca l'Offset del codepoint; si no hi és, cau al glif '?' com a
// substitut. Retorna nullptr si tampoc hi ha '?'.
[[nodiscard]] auto resolveGlyph(std::uint32_t cp) const -> const Offset*;
std::unique_ptr<Sprite> sprite_; std::unique_ptr<Sprite> sprite_;
int box_width_{0}; int box_width_{0};
int box_height_{0}; int box_height_{0};