From 7a14612f0e6d22c4ca35b81d9a37e15b839e23d3 Mon Sep 17 00:00:00 2001 From: scawful Date: Mon, 29 Sep 2025 15:32:37 -0400 Subject: [PATCH] Refactor Tile16Editor and Bitmap palette handling for improved accuracy and stability - Updated Tile16Editor to utilize a fixed height for the scrollable child window, enhancing user interface consistency. - Simplified pixel data handling by removing unnecessary remapping for the display tile, ensuring accurate palette representation. - Enhanced Bitmap palette application logic to directly use the specified index for 8-color palettes, preventing errors in color mapping. - Improved error handling for palette length validation and ensured the complete palette is retained for compatibility with other editors. --- src/app/editor/overworld/tile16_editor.cc | 24 +++------- src/app/gfx/bitmap.cc | 54 ++++++++++++++++------- 2 files changed, 42 insertions(+), 36 deletions(-) diff --git a/src/app/editor/overworld/tile16_editor.cc b/src/app/editor/overworld/tile16_editor.cc index a49cc45a..47461b3b 100644 --- a/src/app/editor/overworld/tile16_editor.cc +++ b/src/app/editor/overworld/tile16_editor.cc @@ -756,12 +756,12 @@ absl::Status Tile16Editor::UpdateTile16Edit() { ImGui::PopStyleColor(3); - // CRITICAL FIX: Use proper scrollable child window instead of gui helpers + // CRITICAL FIX: Use proper scrollable child window with fixed height tile8_source_canvas_.set_draggable(false); // Use direct ImGui child window for proper scrolling if (BeginChild("##Tile8SourceScrollable", - ImVec2(0, ImGui::GetContentRegionAvail().y - 10), true, + ImVec2(0, 32 * 8 * 4), true, ImGuiWindowFlags_AlwaysVerticalScrollbar)) { tile8_source_canvas_.DrawBackground(); @@ -833,25 +833,11 @@ absl::Status Tile16Editor::UpdateTile16Edit() { // Create a display tile that shows the current palette selection gfx::Bitmap display_tile; - // Get the original pixel data + // Get the original pixel data (already has sheet offsets from ProcessGraphicsBuffer) std::vector tile_data = current_gfx_individual_[current_tile8_].vector(); - // Apply palette offset to pixel data (like ZScream does) - // Each pixel value (0-15) needs to be remapped to the correct 8-color sub-palette - if (overworld_palette_.size() >= 256) { - int sheet_index = GetSheetIndexForTile8(current_tile8_); - int actual_palette_slot = GetActualPaletteSlot(current_palette_, sheet_index); - - // Remap pixel indices to the correct palette region - // Pixel values 0-15 in the original data map to different palette slots - for (size_t i = 0; i < tile_data.size(); ++i) { - uint8_t pixel = tile_data[i]; - // Keep only the lower 4 bits (0-15) and add the palette offset - tile_data[i] = (pixel & 0x0F) + actual_palette_slot; - } - } - - // Create the display tile with the remapped pixel data + // The pixel data already contains the correct indices for the 256-color palette + // We don't need to remap - just use it as-is display_tile.Create(8, 8, 8, tile_data); // Apply the complete 256-color palette diff --git a/src/app/gfx/bitmap.cc b/src/app/gfx/bitmap.cc index 2aca181e..9c4c04bb 100644 --- a/src/app/gfx/bitmap.cc +++ b/src/app/gfx/bitmap.cc @@ -352,39 +352,59 @@ void Bitmap::SetPalette(const SnesPalette &palette) { void Bitmap::SetPaletteWithTransparent(const SnesPalette &palette, size_t index, int length) { + if (surface_ == nullptr) { + throw BitmapError("Surface is null. Palette not applied"); + } + + // CRITICAL FIX: Use index directly as palette slot, not index * 7 + // For 8-color palettes, index should be 0-7, not 0-49 if (index >= palette.size()) { throw std::invalid_argument("Invalid palette index"); } - if (length < 0 || length > palette.size()) { - throw std::invalid_argument("Invalid palette length"); + if (length < 0 || length > 8) { + throw std::invalid_argument("Invalid palette length (must be 0-8 for 8-color palettes)"); } if (index + length > palette.size()) { throw std::invalid_argument("Palette index + length exceeds size"); } - if (surface_ == nullptr) { - throw BitmapError("Surface is null. Palette not applied"); - } - - auto start_index = index * 7; - palette_ = palette.sub_palette(start_index, start_index + 7); + // Extract 8-color sub-palette starting at the specified index + // This correctly handles both 256-color overworld palettes and smaller palettes std::vector colors; + + // Always start with transparent color (index 0) colors.push_back(ImVec4(0, 0, 0, 0)); - for (size_t i = start_index; i < start_index + 7; ++i) { - auto &pal_color = palette[i]; + + // Extract up to 7 colors from the palette starting at index + for (size_t i = 0; i < 7 && (index + i) < palette.size(); ++i) { + auto &pal_color = palette[index + i]; colors.push_back(pal_color.rgb()); } + + // Ensure we have exactly 8 colors (transparent + 7 data colors) + while (colors.size() < 8) { + colors.push_back(ImVec4(0, 0, 0, 1.0f)); // Fill with black if needed + } + + // CRITICAL FIX: Keep the original complete palette in palette_ member + // Only update the SDL surface palette for display purposes + // This prevents breaking other editors that expect the complete palette + if (palette_.size() != palette.size()) { + palette_ = palette; // Store complete palette + InvalidatePaletteCache(); // Update cache with complete palette + } + // Apply the 8-color sub-palette to SDL surface for display SDL_UnlockSurface(surface_); - int color_index = 0; - for (const auto &each : colors) { - surface_->format->palette->colors[color_index].r = each.x; - surface_->format->palette->colors[color_index].g = each.y; - surface_->format->palette->colors[color_index].b = each.z; - surface_->format->palette->colors[color_index].a = each.w; - color_index++; + for (int color_index = 0; color_index < 8 && color_index < static_cast(colors.size()); ++color_index) { + if (color_index < surface_->format->palette->ncolors) { + surface_->format->palette->colors[color_index].r = static_cast(colors[color_index].x * 255); + surface_->format->palette->colors[color_index].g = static_cast(colors[color_index].y * 255); + surface_->format->palette->colors[color_index].b = static_cast(colors[color_index].z * 255); + surface_->format->palette->colors[color_index].a = static_cast(colors[color_index].w * 255); + } } SDL_LockSurface(surface_); }