refactor(core/rendering): neteja del parser GIF i construcció de Surface (DUP-01/-06 + HC-03/-10 + PATCH-05)

- DUP-01: les tres branques del switch d'extensions que saltaven el
  mateix (bloc fix + sub-blocs) es substitueixen per una crida a
  skipExtensionBlock(); com que totes les extensions comparteixen
  layout, el switch desapareix i el codi es redueix a una línia.
- DUP-06: el constructor Surface(file_path) llegia el fitxer dues
  voltes (una via loadSurface per pixels i una altra via
  loadPaletteFromGif per la paleta); ara carrega el buffer una sola
  vegada i n'extrau pixels + paleta en passades separades sobre la
  memòria. Surface::loadSurface passa a no existir públicament (no
  tenia callers externs).
- HC-03: les màscares i talls màgics del decoder LZW i del Screen
  Descriptor passen a constants nominals (LZW_MIN/MAX_CODE_LENGTH,
  LZW_MASK_INITIAL, LZW_MASK_OVERFLOW, SCREEN_GCT_FLAG,
  SCREEN_GCT_SIZE_MASK, GCT_BYTES_PER_ENTRY).
- HC-10: processImageDescriptor tenia gct i resolution_bits
  [[maybe_unused]] vestigials; se'n treu la signatura. L'struct RGB,
  que només servia per a aquests args, també s'elimina.
- PATCH-05: el comentari "reemplaçar macro READ" (migració parcial des
  de C) passa a ser un comentari neutre sobre el que fa readBytes.

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:53:43 +02:00
1 parent bbb397abb2
commit be94a4390e
5 files changed
+78 -95

No files matched your search

+7 -6
View File
@@ -30,12 +30,12 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
| ID | Fichero:línea | Hallazgo | Severidad | Estado | Notas |
|----|---------------|----------|-----------|--------|-------|
| 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 | DONE | `skipExtensionBlock` i switch eliminat |
| 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 | 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 |
| DUP-06 | `surface.cpp:14-43` | constructor Surface abre y parsea GIF 2 veces | Importante | DONE | buffer carregat una volta, pixels+palette del mateix |
| DUP-07 | `crtpi_pipeline.hpp:70-87` + `shader_backend.hpp:30-45` + `crtpi_pipeline.cpp:45-62` | defaults CRT-Pi triples | Importante | PENDING | single source of truth |
## 2. Hardcoding
@@ -44,14 +44,14 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
|----|---------------|----------|-----------|--------|-------|
| 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-03 | `gif.cpp:71,95,…` | LZW `<11`/`<12`/`<2` y máscaras `0x80/0x70/0x07…` sin nombre | Menor | DONE | `LZW_MIN/MAX_CODE_LENGTH`, `SCREEN_GCT_FLAG`, `SCREEN_GCT_SIZE_MASK`, etc. |
| 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-05 | `render_info.cpp:19` + `notifier.hpp:43` | `TEXT_SIZE=8` duplicado | Menor | PENDING | constante global font |
| HC-06 | `render_info.cpp:64,138-139,193-195` | `segments_[3]` hardcode del segmento tiempo | Importante | DONE | constants `FPS/DRIVER/SHADER/TIME_SEGMENT` |
| HC-07 | `render_info.cpp:207` | `% 3` hardcode nº de entradas enum | Menor | DONE | `Defaults::RENDER_INFO_POSITION_COUNT` |
| HC-08 | `crtpi_pipeline.cpp:97` | `768.0f / h / 3.0f` magic en shader | Menor | PENDING | uniforme o comentario |
| HC-09 | `render_info.cpp:179-183,194-195` | shadow offset +1,+1 magic | Menor | DONE | `SHADOW_OFFSET_PX` |
| HC-10 | `gif.cpp:162` | `processImageDescriptor` params `[[maybe_unused]]` vestigiales | Menor | PENDING | quitar args |
| HC-10 | `gif.cpp:162` | `processImageDescriptor` params `[[maybe_unused]]` vestigiales | Menor | DONE | signatura sense `gct` ni `resolution_bits`; `RGB` també esborrat |
## 3. Parches e improvisaciones
@@ -61,7 +61,7 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
| PATCH-02 | `render_info.cpp:85-88` | guard bug-fix animation | Menor | PENDING | refactor FSM animación |
| PATCH-03 | `notifier.cpp:78-85` | TODO implícito de cola FIFO | Menor | WONTFIX | documentar decisión |
| PATCH-04 | `sprite.cpp:47-49` | `Sprite::update` vacío hook no invocado | Menor | DONE | resolt amb DEAD-10 |
| PATCH-05 | `gif.cpp:12-15` | comentario "reemplaçar macro READ" | Menor | PENDING | limpiar comentario |
| PATCH-05 | `gif.cpp:12-15` | comentario "reemplaçar macro READ" | Menor | DONE | comentari neutre nou |
| PATCH-06 | `gif.cpp:83-86` | guard self-reference defensivo | Menor | WONTFIX | dejar, cambiar `cerr` → logger futuro |
| PATCH-07 | `sdl3gpu_shader.cpp:17,162` + `crtpi_pipeline.cpp:16,162` | `NOLINTBEGIN` gigantes sobre MSL | Importante | DONE | eliminats amb CONV-03 |
| PATCH-08 | `render_info.cpp:64,138-139` | `mono_digits=true` asignado 2 veces | Menor | DONE | inicialització única via refreshSegments |
@@ -130,4 +130,5 @@ Los IDs se mantienen estables una vez asignados. Cada entrada apunta al(los) fic
- `b51109e` — DEAD-01 + DEAD-02 + DEAD-03: esborrats mètodes mort de `Text`
- `597402a` — DEAD-04/05/07/08/10 + PATCH-04: neteja mort a `Sprite`/`AnimatedSprite`/`Surface` + `Sprite::update` inline
- `ce3b6da` — DEAD-09: esborrats structs d'extensions GIF no usats
- `<pendent>` — HC-06 + HC-07 + HC-09 + PATCH-08: constants nominals a `render_info` (segments, shadow offset, position count) i dedup init
- `bbb397a` — HC-06 + HC-07 + HC-09 + PATCH-08: constants nominals a `render_info` (segments, shadow offset, position count) i dedup init
- `<pendent>` — DUP-01 + DUP-06 + HC-03 + HC-10 + PATCH-05: neteja parser GIF (extension single-skip, constants nominals LZW/GCT, signatura sense args morts, un sol parse per construcció de `Surface`)
+59 -74
View File
@@ -1,19 +1,46 @@
#include "core/rendering/gif.hpp"
#include <cstring> // Para memcpy, size_t
#include <iostream> // Para std::cout
#include <stdexcept> // Para runtime_error
#include <string> // Para allocator, char_traits, operator==, basic_string
#include <cstring>
#include <iostream>
#include <stdexcept>
#include <string>
namespace GIF {
// Función inline para reemplazar el macro READ.
// Actualiza el puntero 'buffer' tras copiar 'size' bytes a 'dst'.
namespace {
// Paràmetres del Screen Descriptor (byte de flags).
constexpr uint8_t SCREEN_GCT_FLAG = 0x80;
constexpr uint8_t SCREEN_GCT_SIZE_MASK = 0x07;
// Paràmetres del decompressor LZW.
constexpr int LZW_MIN_CODE_LENGTH = 2;
constexpr int LZW_MAX_CODE_LENGTH = 12;
constexpr unsigned int LZW_MASK_INITIAL = 0x01;
constexpr unsigned int LZW_MASK_OVERFLOW = 0x100;
// Mida de l'RGB de la GCT (bytes per entrada).
constexpr int GCT_BYTES_PER_ENTRY = 3;
} // namespace
// Copia 'size' bytes del buffer a 'dst' avançant el punter del buffer.
inline void readBytes(const uint8_t*& buffer, void* dst, size_t size) {
std::memcpy(dst, buffer, size);
buffer += size;
}
// Salta un bloc d'extensió: byte de mida + contingut fix + sub-blocs
// acabats amb mida 0. Totes les extensions del GIF (graphic control,
// application, comment, plaintext, ...) comparteixen aquest layout.
inline void skipExtensionBlock(const uint8_t*& buffer) {
uint8_t block_size = *buffer++;
buffer += block_size;
uint8_t sub_block_size = *buffer++;
while (sub_block_size != 0) {
buffer += sub_block_size;
sub_block_size = *buffer++;
}
}
// Inicializa el diccionario LZW con los valores iniciales
inline void initializeDictionary(std::vector<DictionaryEntry>& dictionary, int code_length, int& dictionary_ind) { // NOLINT(readability-identifier-naming)
int size = 1 << code_length;
@@ -35,8 +62,8 @@ namespace GIF {
}
int bit = ((*input & mask) != 0) ? 1 : 0;
mask <<= 1;
if (mask == 0x100) {
mask = 0x01;
if (mask == LZW_MASK_OVERFLOW) {
mask = LZW_MASK_INITIAL;
input++;
input_length--;
}
@@ -68,7 +95,10 @@ namespace GIF {
dictionary[dictionary_ind].len = dictionary[prev].len + 1;
dictionary_ind++;
if ((dictionary_ind == (1 << (code_length + 1))) && (code_length < 11)) {
// Només podem créixer mentre no hagem arribat al màxim del decoder
// (12 bits); el `- 1` compensa el +1 que aplicarem tot seguit per
// a que code_length no sobrepasse LZW_MAX_CODE_LENGTH.
if ((dictionary_ind == (1 << (code_length + 1))) && (code_length < LZW_MAX_CODE_LENGTH - 1)) {
code_length++;
dictionary.resize(1 << (code_length + 1));
}
@@ -92,14 +122,14 @@ namespace GIF {
void Gif::decompress(int code_length, const uint8_t* input, int input_length, uint8_t* out) {
// Verifica que el code_length tenga un rango razonable.
if (code_length < 2 || code_length > 12) {
if (code_length < LZW_MIN_CODE_LENGTH || code_length > LZW_MAX_CODE_LENGTH) {
throw std::runtime_error("Invalid LZW code length");
}
int prev = -1;
std::vector<DictionaryEntry> dictionary;
int dictionary_ind;
unsigned int mask = 0x01;
unsigned int mask = LZW_MASK_INITIAL;
int reset_code_length = code_length;
int clear_code = 1 << code_length;
int stop_code = clear_code + 1;
@@ -123,7 +153,7 @@ namespace GIF {
break;
}
if (prev > -1 && code_length < 12) {
if (prev > -1 && code_length < LZW_MAX_CODE_LENGTH) {
if (code > dictionary_ind) {
std::cerr << "code = " << std::hex << code
<< ", but dictionary_ind = " << dictionary_ind << '\n';
@@ -159,7 +189,7 @@ namespace GIF {
return data;
}
auto Gif::processImageDescriptor(const uint8_t*& buffer, [[maybe_unused]] const std::vector<RGB>& gct, [[maybe_unused]] int resolution_bits) -> std::vector<uint8_t> {
auto Gif::processImageDescriptor(const uint8_t*& buffer) -> std::vector<uint8_t> {
ImageDescriptor image_descriptor;
// Lee 9 bytes para el image descriptor.
readBytes(buffer, &image_descriptor, sizeof(ImageDescriptor));
@@ -185,15 +215,15 @@ namespace GIF {
buffer += sizeof(ScreenDescriptor);
std::vector<uint32_t> global_color_table;
if ((screen_descriptor.fields & 0x80) != 0) {
int global_color_table_size = 1 << ((screen_descriptor.fields & 0x07) + 1);
if ((screen_descriptor.fields & SCREEN_GCT_FLAG) != 0) {
int global_color_table_size = 1 << ((screen_descriptor.fields & SCREEN_GCT_SIZE_MASK) + 1);
global_color_table.resize(global_color_table_size);
for (int i = 0; i < global_color_table_size; ++i) {
uint8_t r = buffer[0];
uint8_t g = buffer[1];
uint8_t b = buffer[2];
global_color_table[i] = (r << 16) | (g << 8) | b;
buffer += 3;
buffer += GCT_BYTES_PER_ENTRY;
}
}
return global_color_table;
@@ -205,79 +235,34 @@ namespace GIF {
std::memcpy(header, buffer, 6);
buffer += 6;
// Opcional: Validar header
std::string header_str(reinterpret_cast<char*>(header), 6);
if (header_str != "GIF87a" && header_str != "GIF89a") {
throw std::runtime_error("Formato de archivo GIF inválido.");
}
// Leer el Screen Descriptor (7 bytes, empaquetado sin padding)
// Screen Descriptor (7 bytes, empaquetado sin padding).
ScreenDescriptor screen_descriptor;
readBytes(buffer, &screen_descriptor, sizeof(ScreenDescriptor));
// Asigna ancho y alto
w = screen_descriptor.width;
h = screen_descriptor.height;
int color_resolution_bits = ((screen_descriptor.fields & 0x70) >> 4) + 1;
std::vector<RGB> global_color_table;
if ((screen_descriptor.fields & 0x80) != 0) {
int global_color_table_size = 1 << ((screen_descriptor.fields & 0x07) + 1);
global_color_table.resize(global_color_table_size);
std::memcpy(global_color_table.data(), buffer, static_cast<size_t>(3) * static_cast<size_t>(global_color_table_size));
buffer += static_cast<ptrdiff_t>(3) * global_color_table_size;
// Saltar la Global Color Table si existeix; els colors els extrau
// loadPalette() en una passada separada sobre el mateix buffer.
if ((screen_descriptor.fields & SCREEN_GCT_FLAG) != 0) {
const int GCT_SIZE = 1 << ((screen_descriptor.fields & SCREEN_GCT_SIZE_MASK) + 1);
buffer += static_cast<ptrdiff_t>(GCT_BYTES_PER_ENTRY) * GCT_SIZE;
}
// Supongamos que 'buffer' es el puntero actual y TRAILER es 0x3B
// Recorregut de blocs fins al TRAILER (0x3B). Les extensions comparteixen
// layout (mida + contingut fix + sub-blocs), per tant totes les saltem
// igual — no cal distingir per extension_label.
uint8_t block_type = *buffer++;
while (block_type != TRAILER) {
if (block_type == EXTENSION_INTRODUCER) // 0x21
{
// Se lee la etiqueta de extensión, la cual indica el tipo de extensión.
uint8_t extension_label = *buffer++;
switch (extension_label) {
case GRAPHIC_CONTROL: // 0xF9
{
// Procesar Graphic Control Extension:
uint8_t block_size = *buffer++; // Normalmente, blockSize == 4
buffer += block_size; // Saltamos los 4 bytes del bloque fijo
// Saltar los sub-bloques
uint8_t sub_block_size = *buffer++;
while (sub_block_size != 0) {
buffer += sub_block_size;
sub_block_size = *buffer++;
}
break;
}
case APPLICATION_EXTENSION: // 0xFF
case COMMENT_EXTENSION: // 0xFE
case PLAINTEXT_EXTENSION: // 0x01
{
// Para estas extensiones, saltamos el bloque fijo y los sub-bloques.
uint8_t block_size = *buffer++;
buffer += block_size;
uint8_t sub_block_size = *buffer++;
while (sub_block_size != 0) {
buffer += sub_block_size;
sub_block_size = *buffer++;
}
break;
}
default: {
// Si la etiqueta de extensión es desconocida, saltarla también:
uint8_t block_size = *buffer++;
buffer += block_size;
uint8_t sub_block_size = *buffer++;
while (sub_block_size != 0) {
buffer += sub_block_size;
sub_block_size = *buffer++;
}
break;
}
}
if (block_type == EXTENSION_INTRODUCER) {
++buffer; // extension_label (ignorat)
skipExtensionBlock(buffer);
} else if (block_type == IMAGE_DESCRIPTOR) {
// Procesar el Image Descriptor y retornar los datos de imagen
return processImageDescriptor(buffer, global_color_table, color_resolution_bits);
return processImageDescriptor(buffer);
} else {
std::cerr << "Unrecognized block type " << std::hex << static_cast<int>(block_type) << '\n';
return std::vector<uint8_t>{};
+1 -5
View File
@@ -23,10 +23,6 @@ namespace GIF {
uint8_t pixel_aspect_ratio;
};
struct RGB {
uint8_t r, g, b;
};
struct ImageDescriptor {
uint16_t image_left_position;
uint16_t image_top_position;
@@ -61,7 +57,7 @@ namespace GIF {
static auto readSubBlocks(const uint8_t*& buffer) -> std::vector<uint8_t>;
// Procesa el Image Descriptor y retorna el vector de datos sin comprimir.
static auto processImageDescriptor(const uint8_t*& buffer, const std::vector<RGB>& gct, int resolution_bits) -> std::vector<uint8_t>;
static auto processImageDescriptor(const uint8_t*& buffer) -> std::vector<uint8_t>;
// Procesa el stream completo del GIF y devuelve los datos sin comprimir.
static auto processGifStream(const uint8_t* buffer, uint16_t& w, uint16_t& h) -> std::vector<uint8_t>;
+11 -8
View File
@@ -37,13 +37,9 @@ Surface::Surface(int w, int h)
}
Surface::Surface(const std::string& file_path) {
SurfaceData loaded = loadSurface(file_path);
surface_data_ = std::make_shared<SurfaceData>(std::move(loaded));
initializeSubPalette(sub_palette_);
palette_ = loadPaletteFromGif(file_path);
}
auto Surface::loadSurface(const std::string& file_path) -> SurfaceData {
// Carreguem el fitxer una única vegada i n'extraiem pixels i paleta de
// passades independents sobre el mateix buffer en memòria — abans es
// parsejava dues voltes (loadSurface + loadPaletteFromGif).
auto buffer = Resource::Helper::loadFile(file_path);
if (buffer.empty()) {
throw std::runtime_error("Surface: cannot open " + file_path);
@@ -58,9 +54,16 @@ auto Surface::loadSurface(const std::string& file_path) -> SurfaceData {
auto pixels = std::shared_ptr<Uint8[]>(new Uint8[raw_pixels.size()], std::default_delete<Uint8[]>());
std::memcpy(pixels.get(), raw_pixels.data(), raw_pixels.size());
surface_data_ = std::make_shared<SurfaceData>(static_cast<int>(w), static_cast<int>(h), pixels);
initializeSubPalette(sub_palette_);
std::vector<std::uint32_t> rgb = GIF::Gif::loadPalette(buffer.data());
const std::size_t COUNT = std::min(rgb.size(), palette_.size());
for (std::size_t i = 0; i < COUNT; ++i) {
palette_[i] = 0xFF000000U | rgb[i];
}
printWithDots("Surface : ", getFileName(file_path), "[ LOADED ]");
return {static_cast<int>(w), static_cast<int>(h), pixels};
}
void Surface::clear(Uint8 color) {
-2
View File
@@ -45,8 +45,6 @@ class Surface {
explicit Surface(const std::string& file_path);
~Surface() = default;
static auto loadSurface(const std::string& file_path) -> SurfaceData;
// Vobla la regió indicada al framebuffer ARGB del joc usant la paleta propia.
void render(int x, int y, const SDL_FRect* src_rect = nullptr, SDL_FlipMode flip = SDL_FLIP_NONE) const;
// Igual que render() però substituint els píxels amb valor source_color per