refactor: Improve Rendering Logic and Debugging in Dungeon Components
- Updated DungeonCanvasViewer to disable room layout drawing to reduce visual clutter, enhancing clarity during object placement. - Enhanced BackgroundBuffer to skip empty and floor tiles, preventing overwriting of drawn elements and improving rendering efficiency. - Refined Bitmap palette application to ensure correct transparency handling and marked bitmaps as modified for texture updates. - Streamlined ObjectDrawer by removing unnecessary debug logs and simplifying object drawing logic, improving performance and readability. - Adjusted Room rendering methods to utilize palette indirection for accurate color application, ensuring consistent visual output across rooms.
This commit is contained in:
@@ -519,13 +519,15 @@ void DungeonCanvasViewer::DrawDungeonCanvas(int room_id) {
|
||||
}
|
||||
}
|
||||
|
||||
// Draw layout overlays on top of background bitmap
|
||||
// Draw optional overlays on top of background bitmap
|
||||
if (rooms_ && rom_->is_loaded()) {
|
||||
auto& room = (*rooms_)[room_id];
|
||||
|
||||
// Draw room layout (structural elements like walls, pits)
|
||||
// This provides context for object placement
|
||||
DrawRoomLayout(room);
|
||||
// DISABLED: Room layout drawing - causes visual clutter
|
||||
// Layout tiles (2793) render over everything and obscure objects
|
||||
// if (show_layout_overlay_) {
|
||||
// DrawRoomLayout(room);
|
||||
// }
|
||||
|
||||
// VISUALIZATION: Draw object position rectangles (for debugging)
|
||||
// This shows where objects are placed regardless of whether graphics render
|
||||
|
||||
@@ -106,22 +106,29 @@ void BackgroundBuffer::DrawBackground(std::span<uint8_t> gfx16_data) {
|
||||
// For each tile on the tile buffer
|
||||
int drawn_count = 0;
|
||||
int skipped_count = 0;
|
||||
int non_floor_count = 0;
|
||||
for (int yy = 0; yy < tiles_h; yy++) {
|
||||
for (int xx = 0; xx < tiles_w; xx++) {
|
||||
uint16_t word = buffer_[xx + yy * tiles_w];
|
||||
// Prevent draw if tile == 0xFFFF since it's 0 indexed
|
||||
|
||||
// Skip empty tiles (0xFFFF) - these show the floor
|
||||
if (word == 0xFFFF) {
|
||||
skipped_count++;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Skip zero tiles - also show the floor
|
||||
if (word == 0) {
|
||||
skipped_count++;
|
||||
continue;
|
||||
}
|
||||
|
||||
auto tile = gfx::WordToTileInfo(word);
|
||||
|
||||
// Count floor tiles vs non-floor
|
||||
if (tile.id_ >= 0xEE && tile.id_ <= 0xFF) {
|
||||
// This looks like a floor tile (based on DrawFloor's 14EE pattern)
|
||||
} else if (word != 0) {
|
||||
non_floor_count++;
|
||||
// Skip floor tiles (0xEC-0xFD) - don't overwrite DrawFloor's work
|
||||
// These are the animated floor tiles, already drawn by DrawFloor
|
||||
if (tile.id_ >= 0xEC && tile.id_ <= 0xFD) {
|
||||
skipped_count++;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Calculate pixel offset for tile position (xx, yy) in the 512x512 bitmap
|
||||
@@ -176,6 +183,9 @@ void BackgroundBuffer::DrawFloor(const std::vector<uint8_t>& rom_data,
|
||||
rom_data[tile_address_floor + f + 5]);
|
||||
gfx::TileInfo floorTile8(rom_data[tile_address_floor + f + 6],
|
||||
rom_data[tile_address_floor + f + 7]);
|
||||
|
||||
// Floor tiles specify which 8-color sub-palette from the 90-color dungeon palette
|
||||
// e.g., palette 6 = colors 48-55 (6 * 8 = 48)
|
||||
|
||||
// Draw the floor tiles in a pattern
|
||||
// Convert TileInfo to 16-bit words with palette information
|
||||
|
||||
@@ -269,13 +269,23 @@ void Bitmap::ApplyStoredPalette() {
|
||||
}
|
||||
|
||||
SDL_UnlockSurface(surface_);
|
||||
|
||||
// Apply all palette colors from the SnesPalette
|
||||
for (size_t i = 0; i < palette_.size() && i < 256; ++i) {
|
||||
const auto& pal_color = palette_[i];
|
||||
sdl_palette->colors[i].r = pal_color.rgb().x;
|
||||
sdl_palette->colors[i].g = pal_color.rgb().y;
|
||||
sdl_palette->colors[i].b = pal_color.rgb().z;
|
||||
sdl_palette->colors[i].a = pal_color.rgb().w;
|
||||
// NOTE: rgb() stores 0-255 values directly in ImVec4 (unconventional but intentional)
|
||||
sdl_palette->colors[i].r = static_cast<Uint8>(pal_color.rgb().x);
|
||||
sdl_palette->colors[i].g = static_cast<Uint8>(pal_color.rgb().y);
|
||||
sdl_palette->colors[i].b = static_cast<Uint8>(pal_color.rgb().z);
|
||||
|
||||
// CRITICAL: Transparency for color 0 of each sub-palette
|
||||
if (pal_color.is_transparent()) {
|
||||
sdl_palette->colors[i].a = 0; // Fully transparent
|
||||
} else {
|
||||
sdl_palette->colors[i].a = 255; // Fully opaque
|
||||
}
|
||||
}
|
||||
|
||||
SDL_LockSurface(surface_);
|
||||
}
|
||||
|
||||
@@ -285,6 +295,9 @@ void Bitmap::SetPalette(const SnesPalette &palette) {
|
||||
|
||||
// Apply it immediately if surface is ready
|
||||
ApplyStoredPalette();
|
||||
|
||||
// Mark as modified to trigger texture update
|
||||
modified_ = true;
|
||||
}
|
||||
|
||||
void Bitmap::SetPaletteWithTransparent(const SnesPalette &palette, size_t index,
|
||||
|
||||
@@ -28,29 +28,17 @@ absl::Status ObjectDrawer::DrawObject(const RoomObject& object,
|
||||
|
||||
// Ensure object has tiles loaded
|
||||
auto mutable_obj = const_cast<RoomObject&>(object);
|
||||
LOG_DEBUG("ObjectDrawer", "Setting ROM for object ID=0x%02X", object.id_);
|
||||
mutable_obj.set_rom(rom_);
|
||||
LOG_DEBUG("ObjectDrawer", "Calling EnsureTilesLoaded for object ID=0x%02X", object.id_);
|
||||
mutable_obj.EnsureTilesLoaded();
|
||||
|
||||
// Check if tiles were actually loaded on the mutable object
|
||||
LOG_DEBUG("ObjectDrawer", "After EnsureTilesLoaded: mutable object has %zu tiles",
|
||||
mutable_obj.tiles().size());
|
||||
|
||||
// Select buffer based on layer
|
||||
auto& target_bg = (object.layer_ == RoomObject::LayerType::BG2) ? bg2 : bg1;
|
||||
LOG_DEBUG("ObjectDrawer", "Object ID=0x%02X using %s buffer (layer=%d)",
|
||||
object.id_, (object.layer_ == RoomObject::LayerType::BG2) ? "BG2" : "BG1", object.layer_);
|
||||
|
||||
// Skip objects that don't have tiles loaded - check mutable object
|
||||
if (mutable_obj.tiles().empty()) {
|
||||
LOG_DEBUG("ObjectDrawer", "Object ID=0x%02X has no tiles loaded, skipping", object.id_);
|
||||
return absl::OkStatus();
|
||||
}
|
||||
|
||||
LOG_DEBUG("ObjectDrawer", "Object ID=0x%02X has %zu tiles, proceeding with drawing",
|
||||
object.id_, mutable_obj.tiles().size());
|
||||
|
||||
// Look up draw routine for this object
|
||||
int routine_id = GetDrawRoutineId(object.id_);
|
||||
|
||||
@@ -75,27 +63,8 @@ absl::Status ObjectDrawer::DrawObjectList(
|
||||
gfx::BackgroundBuffer& bg2,
|
||||
const gfx::PaletteGroup& palette_group) {
|
||||
|
||||
LOG_DEBUG("ObjectDrawer", "Drawing %zu objects", objects.size());
|
||||
|
||||
int drawn_count = 0;
|
||||
int skipped_count = 0;
|
||||
|
||||
for (const auto& object : objects) {
|
||||
auto status = DrawObject(object, bg1, bg2, palette_group);
|
||||
if (status.ok()) {
|
||||
drawn_count++;
|
||||
} else {
|
||||
skipped_count++;
|
||||
// Only print errors that aren't "no tiles" (which is common and expected)
|
||||
if (status.code() != absl::StatusCode::kOk) {
|
||||
// Skip silently - many objects don't have tiles loaded yet
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Only log if there are failures
|
||||
if (skipped_count > 0) {
|
||||
LOG_DEBUG("ObjectDrawer", "Drew %d objects, skipped %d", drawn_count, skipped_count);
|
||||
DrawObject(object, bg1, bg2, palette_group);
|
||||
}
|
||||
|
||||
return absl::OkStatus();
|
||||
@@ -663,14 +632,9 @@ void ObjectDrawer::DrawRightwardsDecor2x2spaced12_1to16(const RoomObject& obj, g
|
||||
|
||||
void ObjectDrawer::WriteTile8(gfx::BackgroundBuffer& bg, int tile_x, int tile_y,
|
||||
const gfx::TileInfo& tile_info) {
|
||||
LOG_DEBUG("ObjectDrawer", "Writing 8x8 tile at tile pos (%d,%d) to bitmap", tile_x, tile_y);
|
||||
|
||||
// Draw directly to bitmap instead of tile buffer to avoid being overwritten
|
||||
auto& bitmap = bg.bitmap();
|
||||
LOG_DEBUG("ObjectDrawer", "Bitmap status: active=%d, width=%d, height=%d, surface=%p",
|
||||
bitmap.is_active(), bitmap.width(), bitmap.height(), bitmap.surface());
|
||||
if (!bitmap.is_active() || bitmap.width() == 0) {
|
||||
LOG_DEBUG("ObjectDrawer", "Bitmap not ready: active=%d, width=%d", bitmap.is_active(), bitmap.width());
|
||||
return; // Bitmap not ready
|
||||
}
|
||||
|
||||
@@ -714,20 +678,20 @@ void ObjectDrawer::DrawTileToBitmap(gfx::Bitmap& bitmap, const gfx::TileInfo& ti
|
||||
return;
|
||||
}
|
||||
|
||||
// Calculate tile position in graphics sheet (128 pixels wide)
|
||||
// Calculate tile position in graphics sheet (128 pixels wide, 16 tiles per row)
|
||||
int tile_sheet_x = (tile_info.id_ % 16) * 8; // 16 tiles per row
|
||||
int tile_sheet_y = (tile_info.id_ / 16) * 8; // Each row is 16 tiles
|
||||
|
||||
// Clamp palette to valid range
|
||||
uint8_t palette_id = tile_info.palette_ & 0x0F;
|
||||
if (palette_id > 10) palette_id = palette_id % 11;
|
||||
uint8_t palette_offset = palette_id * 8; // 3BPP: 8 colors per palette
|
||||
// CRITICAL: Dungeon palettes are 90 colors organized as:
|
||||
// - 10 sub-palettes of 8 colors each (0-79)
|
||||
// - Plus 10 additional colors (80-89)
|
||||
// Each tile uses palette field (0-10) to select which 8-color sub-palette
|
||||
uint8_t palette_id = tile_info.palette_ & 0x0F; // 0-15 possible
|
||||
if (palette_id > 10) palette_id = 10; // Clamp to 0-10 for dungeons
|
||||
|
||||
// Force a visible palette for debugging
|
||||
if (palette_id == 0) {
|
||||
palette_id = 1; // Use palette 1 instead of 0
|
||||
palette_offset = palette_id * 8;
|
||||
}
|
||||
// Calculate color offset in dungeon palette
|
||||
// For dungeon: palette 0 = colors 0-7, palette 1 = colors 8-15, etc.
|
||||
uint8_t palette_offset = palette_id * 8;
|
||||
|
||||
// Draw 8x8 pixels
|
||||
for (int py = 0; py < 8; py++) {
|
||||
@@ -749,12 +713,6 @@ void ObjectDrawer::DrawTileToBitmap(gfx::Bitmap& bitmap, const gfx::TileInfo& ti
|
||||
int dest_index = dest_y * bitmap.width() + dest_x;
|
||||
if (dest_index >= 0 && dest_index < static_cast<int>(bitmap.mutable_data().size())) {
|
||||
bitmap.mutable_data()[dest_index] = final_color;
|
||||
|
||||
// Debug first pixel of each tile
|
||||
if (py == 0 && px == 0) {
|
||||
LOG_DEBUG("ObjectDrawer", "Tile ID=0x%02X at (%d,%d): palette=%d, pixel_index=%d, final_color=%d",
|
||||
tile_info.id_, pixel_x, pixel_y, palette_id, pixel_index, final_color);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -312,36 +312,29 @@ void Room::RenderRoomGraphics() {
|
||||
// Get and apply palette BEFORE rendering objects (so objects use correct colors)
|
||||
auto& dungeon_pal_group = rom()->mutable_palette_group()->dungeon_main;
|
||||
int num_palettes = dungeon_pal_group.size();
|
||||
int palette_id = palette;
|
||||
|
||||
LOG_DEBUG("[RenderRoomGraphics]", "Room palette byte: %d, num_palettes available: %d",
|
||||
palette_id, num_palettes);
|
||||
// Use palette indirection table lookup (same as dungeon_canvas_viewer.cc line 854)
|
||||
int palette_id = palette; // Default fallback
|
||||
if (palette < rom()->paletteset_ids.size() && !rom()->paletteset_ids[palette].empty()) {
|
||||
auto dungeon_palette_ptr = rom()->paletteset_ids[palette][0];
|
||||
auto palette_word = rom()->ReadWord(0xDEC4B + dungeon_palette_ptr);
|
||||
if (palette_word.ok()) {
|
||||
palette_id = palette_word.value() / 180; // Divide by 180 to get group index
|
||||
LOG_DEBUG("[RenderRoomGraphics]", "Palette lookup: byte=0x%02X → group_id=%d", palette, palette_id);
|
||||
}
|
||||
}
|
||||
|
||||
// Clamp to valid range
|
||||
if (palette_id < 0 || palette_id >= num_palettes) {
|
||||
LOG_DEBUG("[RenderRoomGraphics]", "WARNING: palette_id %d out of bounds, clamping to %d",
|
||||
palette_id, palette_id % num_palettes);
|
||||
palette_id = palette_id % num_palettes;
|
||||
}
|
||||
|
||||
auto bg1_palette = dungeon_pal_group[palette_id];
|
||||
|
||||
if (bg1_palette.size() > 0) {
|
||||
LOG_DEBUG("[RenderRoomGraphics]", "Applying palette_id=%d with %d colors to bitmaps",
|
||||
palette_id, (int)bg1_palette.size());
|
||||
|
||||
// Debug: Log first few palette colors
|
||||
for (int i = 0; i < std::min(16, (int)bg1_palette.size()); i++) {
|
||||
auto rgb = bg1_palette[i].rgb();
|
||||
LOG_DEBUG("[RenderRoomGraphics]", " Palette[%d]: R=%.0f G=%.0f B=%.0f A=%.0f %s",
|
||||
i, rgb.x, rgb.y, rgb.z, rgb.w,
|
||||
bg1_palette[i].is_transparent() ? "(TRANSPARENT)" : "");
|
||||
}
|
||||
|
||||
// Apply FULL 90-color dungeon palette
|
||||
bg1_bmp.SetPalette(bg1_palette);
|
||||
bg2_bmp.SetPalette(bg1_palette);
|
||||
} else {
|
||||
LOG_DEBUG("[RenderRoomGraphics]", "ERROR: Palette is empty!");
|
||||
}
|
||||
|
||||
// Render objects ON TOP of background tiles (AFTER palette is set)
|
||||
@@ -379,10 +372,19 @@ void Room::RenderObjectsToBackground() {
|
||||
return;
|
||||
}
|
||||
|
||||
// Get palette group for object rendering
|
||||
// Get palette group for object rendering (use SAME lookup as RenderRoomGraphics)
|
||||
auto& dungeon_pal_group = rom()->mutable_palette_group()->dungeon_main;
|
||||
int num_palettes = dungeon_pal_group.size();
|
||||
|
||||
// Use palette indirection table lookup
|
||||
int palette_id = palette;
|
||||
if (palette < rom()->paletteset_ids.size() && !rom()->paletteset_ids[palette].empty()) {
|
||||
auto dungeon_palette_ptr = rom()->paletteset_ids[palette][0];
|
||||
auto palette_word = rom()->ReadWord(0xDEC4B + dungeon_palette_ptr);
|
||||
if (palette_word.ok()) {
|
||||
palette_id = palette_word.value() / 180;
|
||||
}
|
||||
}
|
||||
|
||||
if (palette_id < 0 || palette_id >= num_palettes) {
|
||||
palette_id = 0;
|
||||
|
||||
@@ -50,28 +50,20 @@ ObjectOption operator~(ObjectOption option) {
|
||||
// Modern rendering uses ObjectDrawer which draws directly to BackgroundBuffer bitmaps.
|
||||
|
||||
void RoomObject::EnsureTilesLoaded() {
|
||||
LOG_DEBUG("RoomObject", "Object ID=0x%02X, tiles_loaded=%d", id_, tiles_loaded_);
|
||||
|
||||
if (tiles_loaded_) {
|
||||
LOG_DEBUG("RoomObject", "Tiles already loaded for object 0x%02X", id_);
|
||||
return;
|
||||
}
|
||||
|
||||
if (rom_ == nullptr) {
|
||||
LOG_DEBUG("RoomObject", "ERROR: ROM not set for object 0x%02X", id_);
|
||||
return;
|
||||
}
|
||||
|
||||
// Try the new parser first - this is more efficient and accurate
|
||||
LOG_DEBUG("RoomObject", "Trying parser for object 0x%02X", id_);
|
||||
auto parser_status = LoadTilesWithParser();
|
||||
if (parser_status.ok()) {
|
||||
LOG_DEBUG("RoomObject", "Parser succeeded for object 0x%02X, loaded %zu tiles", id_, tiles_.size());
|
||||
tiles_loaded_ = true;
|
||||
return;
|
||||
}
|
||||
|
||||
LOG_DEBUG("RoomObject", "Parser failed for object 0x%02X: %s", id_, parser_status.message().data());
|
||||
|
||||
// Fallback to legacy method for compatibility with enhanced validation
|
||||
auto rom_data = rom_->data();
|
||||
|
||||
Reference in New Issue
Block a user