diff --git a/docs/audit-core-rendering.md b/docs/audit-core-rendering.md index 3d8f1eb..123d102 100644 --- a/docs/audit-core-rendering.md +++ b/docs/audit-core-rendering.md @@ -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 -- `` — 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 +- `` — 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`) diff --git a/source/core/rendering/gif.cpp b/source/core/rendering/gif.cpp index e39cd7f..c3182f1 100644 --- a/source/core/rendering/gif.cpp +++ b/source/core/rendering/gif.cpp @@ -1,19 +1,46 @@ #include "core/rendering/gif.hpp" -#include // Para memcpy, size_t -#include // Para std::cout -#include // Para runtime_error -#include // Para allocator, char_traits, operator==, basic_string +#include +#include +#include +#include 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& 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 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& gct, [[maybe_unused]] int resolution_bits) -> std::vector { + auto Gif::processImageDescriptor(const uint8_t*& buffer) -> std::vector { 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 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(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 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(3) * static_cast(global_color_table_size)); - buffer += static_cast(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(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(block_type) << '\n'; return std::vector{}; diff --git a/source/core/rendering/gif.hpp b/source/core/rendering/gif.hpp index c1e30a6..dc52f70 100644 --- a/source/core/rendering/gif.hpp +++ b/source/core/rendering/gif.hpp @@ -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; // Procesa el Image Descriptor y retorna el vector de datos sin comprimir. - static auto processImageDescriptor(const uint8_t*& buffer, const std::vector& gct, int resolution_bits) -> std::vector; + static auto processImageDescriptor(const uint8_t*& buffer) -> std::vector; // 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; diff --git a/source/core/rendering/surface.cpp b/source/core/rendering/surface.cpp index 100c345..10768c9 100644 --- a/source/core/rendering/surface.cpp +++ b/source/core/rendering/surface.cpp @@ -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(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(new Uint8[raw_pixels.size()], std::default_delete()); std::memcpy(pixels.get(), raw_pixels.data(), raw_pixels.size()); + surface_data_ = std::make_shared(static_cast(w), static_cast(h), pixels); + initializeSubPalette(sub_palette_); + + std::vector 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(w), static_cast(h), pixels}; } void Surface::clear(Uint8 color) { diff --git a/source/core/rendering/surface.hpp b/source/core/rendering/surface.hpp index ee1aa3b..b721202 100644 --- a/source/core/rendering/surface.hpp +++ b/source/core/rendering/surface.hpp @@ -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