From a63deac6a3a3eaeb076c426db4137b35dd0cbb1c Mon Sep 17 00:00:00 2001 From: Sergio Valor Date: Mon, 20 Apr 2026 09:32:37 +0200 Subject: [PATCH] refactor(core/rendering): deduplicar Text::write* i nomenar magic numbers (DUP-03 + HC-01) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/audit-core-rendering.md | 9 ++-- source/core/rendering/text.cpp | 80 +++++++++++++--------------------- source/core/rendering/text.hpp | 11 +++++ 3 files changed, 47 insertions(+), 53 deletions(-) diff --git a/docs/audit-core-rendering.md b/docs/audit-core-rendering.md index bea2f77..684e399 100644 --- a/docs/audit-core-rendering.md +++ b/docs/audit-core-rendering.md @@ -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 | | DUP-02 | `Surface::render` / `renderWithColorReplace` 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 | --- @@ -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-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-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 | @@ -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 | |----|---------------|----------|-----------|--------|-------| -| 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-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 | @@ -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` - `9395520` — COUP-01b: `RenderInfo` desacoblat de `game/options` + `game/ui/ui_palette` - `d479475` — DUP-02: `Surface::render` delega a `renderWithColorReplace` -- `` — 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` +- `` — DUP-03 + HC-01: `Text` amb `resolveGlyph` i constants `INK_COLOR_INDEX` / `DEFAULT_TEXT_COLOR` diff --git a/source/core/rendering/text.cpp b/source/core/rendering/text.cpp index 04c1ed1..1f40c73 100644 --- a/source/core/rendering/text.cpp +++ b/source/core/rendering/text.cpp @@ -140,27 +140,10 @@ Text::Text(const std::shared_ptr& surface, const std::shared_ptr& offset_(text_file->offset) { } -// Dibuixa la cadena amb la tinta per defecte (codi 15 a la paleta del png). -// Els glifs amb codepoint no registrat es substitueixen pel glif '?'. +// Dibuixa la cadena amb la tinta per defecte. Els glifs amb codepoint no +// registrat es substitueixen pel glif '?'. void Text::write(int x, int y, const std::string& text, int kerning, int length) { - int shift = 0; - int glyphs_done = 0; - std::size_t pos = 0; - - sprite_->setY(static_cast(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(it->second.x), static_cast(it->second.y), static_cast(box_width_), static_cast(box_height_)); - sprite_->setX(static_cast(x + shift)); - sprite_->render(1, 15); - shift += it->second.w + kerning; - } - ++glyphs_done; - } + writeColored(x, y, text, DEFAULT_TEXT_COLOR, kerning, 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()) { 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(it->second.x), static_cast(it->second.y), static_cast(box_width_), static_cast(box_height_)); + if (const Offset* off = resolveGlyph(cp); off != nullptr) { + sprite_->setClip(static_cast(off->x), static_cast(off->y), static_cast(box_width_), static_cast(box_height_)); sprite_->setX(static_cast(x + shift)); - sprite_->render(1, color); - shift += it->second.w + kerning; + sprite_->render(INK_COLOR_INDEX, color); + shift += off->w + kerning; } ++glyphs_done; } @@ -235,21 +216,20 @@ void Text::writeMonoDigits(int x, int y, const std::string& text, Uint8 color, i sprite_->setY(static_cast(y)); while (pos < text.size()) { std::uint32_t cp = nextCodepoint(text, pos); - auto it = offset_.find(cp); - if (it == offset_.end()) { it = offset_.find('?'); } - if (it == offset_.end()) { continue; } + const Offset* off = resolveGlyph(cp); + if (off == nullptr) { continue; } 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 GLYPH_X = IS_DIGIT ? x + shift + ((CELL_W - GLYPH_W) / 2) : x + shift; - sprite_->setClip(static_cast(it->second.x), - static_cast(it->second.y), + sprite_->setClip(static_cast(off->x), + static_cast(off->y), static_cast(box_width_), static_cast(box_height_)); sprite_->setX(static_cast(GLYPH_X)); - sprite_->render(1, color); + sprite_->render(INK_COLOR_INDEX, color); 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()) { std::uint32_t cp = nextCodepoint(text, pos); - auto it = offset_.find(cp); - if (it == offset_.end()) { it = offset_.find('?'); } - if (it == offset_.end()) { continue; } + const Offset* off = resolveGlyph(cp); + if (off == nullptr) { continue; } 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; @@ -278,10 +257,8 @@ auto Text::length(const std::string& text, int kerning) const -> int { while (pos < text.size()) { std::uint32_t cp = nextCodepoint(text, pos); - auto it = offset_.find(cp); - if (it == offset_.end()) { it = offset_.find('?'); } - if (it != offset_.end()) { - shift += it->second.w + kerning; + if (const Offset* off = resolveGlyph(cp); off != nullptr) { + shift += off->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 it = offset_.find(codepoint); - if (it == offset_.end()) { it = offset_.find('?'); } - if (it != offset_.end()) { return it->second.w + kerning; } + if (const Offset* off = resolveGlyph(codepoint); off != nullptr) { + return off->w + kerning; + } return 0; } auto Text::getGlyphClip(std::uint32_t codepoint) const -> SDL_FRect { - auto it = offset_.find(codepoint); - if (it == offset_.end()) { it = offset_.find('?'); } - if (it == offset_.end()) { return {.x = 0.0F, .y = 0.0F, .w = 0.0F, .h = 0.0F}; } - return {.x = static_cast(it->second.x), - .y = static_cast(it->second.y), + const Offset* off = resolveGlyph(codepoint); + if (off == nullptr) { return {.x = 0.0F, .y = 0.0F, .w = 0.0F, .h = 0.0F}; } + return {.x = static_cast(off->x), + .y = static_cast(off->y), .w = static_cast(box_width_), .h = static_cast(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; +} diff --git a/source/core/rendering/text.hpp b/source/core/rendering/text.hpp index f0e68fb..a65c023 100644 --- a/source/core/rendering/text.hpp +++ b/source/core/rendering/text.hpp @@ -36,6 +36,13 @@ class Text { static constexpr Uint8 CENTER_FLAG = 4; 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, const std::string& text_file); Text(const std::shared_ptr& surface, const std::shared_ptr& text_file); ~Text() = default; @@ -68,6 +75,10 @@ class Text { static auto nextCodepoint(const std::string& s, std::size_t& pos) -> std::uint32_t; 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_; int box_width_{0}; int box_height_{0};