Pica/Texture: Simplify/cleanup texture tile addressing

src/citra_qt/debugger/graphics/graphics_surface.cpp

@@ -568,19 +568,14 @@ void GraphicsSurfaceWidget::OnUpdate() {
 
     surface_picture_label->show();
 
-    unsigned nibbles_per_pixel = GraphicsSurfaceWidget::NibblesPerPixel(surface_format);
-    unsigned stride = nibbles_per_pixel * surface_width / 2;
-
-    // We handle depth formats here because DebugUtils only supports TextureFormats
     if (surface_format <= Format::MaxTextureFormat) {
-
         // Generate a virtual texture
         Pica::Texture::TextureInfo info;
         info.physical_address = surface_address;
         info.width = surface_width;
         info.height = surface_height;
         info.format = static_cast<Pica::Regs::TextureFormat>(surface_format);
-        info.stride = stride;
+        info.SetDefaultStride();
 
         for (unsigned int y = 0; y < surface_height; ++y) {
             for (unsigned int x = 0; x < surface_width; ++x) {
@@ -588,8 +583,12 @@ void GraphicsSurfaceWidget::OnUpdate() {
                 decoded_image.setPixel(x, y, qRgba(color.r(), color.g(), color.b(), color.a()));
             }
         }
-
     } else {
+        // We handle depth formats here because DebugUtils only supports TextureFormats
+
+        // TODO(yuriks): Convert to newer tile-based addressing
+        unsigned nibbles_per_pixel = GraphicsSurfaceWidget::NibblesPerPixel(surface_format);
+        unsigned stride = nibbles_per_pixel * surface_width / 2;
 
         ASSERT_MSG(nibbles_per_pixel >= 2,
                    "Depth decoder only supports formats with at least one byte per pixel");

src/video_core/pica.h

@@ -275,8 +275,11 @@ struct Regs {
         case TextureFormat::I8:
         case TextureFormat::A8:
         case TextureFormat::IA4:
-        default: // placeholder for yet unknown formats
             return 2;
+
+        default: // placeholder for yet unknown formats
+            UNIMPLEMENTED();
+            return 0;
         }
     }
 

src/video_core/renderer_opengl/gl_rasterizer_cache.cpp

@@ -342,9 +342,8 @@ CachedSurface* RasterizerCacheOpenGL::GetSurface(const CachedSurface& params, bo
                 Pica::Texture::TextureInfo tex_info;
                 tex_info.width = params.width;
                 tex_info.height = params.height;
-                tex_info.stride =
-                    params.width * CachedSurface::GetFormatBpp(params.pixel_format) / 8;
                 tex_info.format = (Pica::Regs::TextureFormat)params.pixel_format;
+                tex_info.SetDefaultStride();
                 tex_info.physical_address = params.addr;
 
                 for (unsigned y = 0; y < params.height; ++y) {

src/video_core/texture/texture_decode.cpp

@@ -9,52 +9,104 @@
 #include "common/logging/log.h"
 #include "common/math_util.h"
 #include "common/vector_math.h"
+#include "video_core/pica.h"
 #include "video_core/texture/texture_decode.h"
 #include "video_core/utils.h"
 
+using TextureFormat = Pica::Regs::TextureFormat;
+
 namespace Pica {
 namespace Texture {
 
-Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo& info,
+constexpr size_t TILE_SIZE = 8 * 8;
-                             bool disable_alpha) {
+constexpr size_t ETC1_SUBTILES = 2 * 2;
-    const unsigned int coarse_x = x & ~7;
-    const unsigned int coarse_y = y & ~7;
 
-    if (info.format != Regs::TextureFormat::ETC1 && info.format != Regs::TextureFormat::ETC1A4) {
+size_t CalculateTileSize(TextureFormat format) {
-        // TODO(neobrain): Fix code design to unify vertical block offsets!
+    switch (format) {
-        source += coarse_y * info.stride;
+    case TextureFormat::RGBA8:
+        return 4 * TILE_SIZE;
+
+    case TextureFormat::RGB8:
+        return 3 * TILE_SIZE;
+
+    case TextureFormat::RGB5A1:
+    case TextureFormat::RGB565:
+    case TextureFormat::RGBA4:
+    case TextureFormat::IA8:
+    case TextureFormat::RG8:
+        return 2 * TILE_SIZE;
+
+    case TextureFormat::I8:
+    case TextureFormat::A8:
+    case TextureFormat::IA4:
+        return 1 * TILE_SIZE;
+
+    case TextureFormat::I4:
+    case TextureFormat::A4:
+        return TILE_SIZE / 2;
+
+    case TextureFormat::ETC1:
+        return ETC1_SUBTILES * 8;
+
+    case TextureFormat::ETC1A4:
+        return ETC1_SUBTILES * 16;
+
+    default: // placeholder for yet unknown formats
+        UNIMPLEMENTED();
+        return 0;
+    }
 }
 
-    // TODO: Assert that width/height are multiples of block dimensions
+Math::Vec4<u8> LookupTexture(const u8* source, unsigned int x, unsigned int y,
+                             const TextureInfo& info, bool disable_alpha) {
+    // Coordinate in tiles
+    const unsigned int coarse_x = x / 8;
+    const unsigned int coarse_y = y / 8;
+
+    // Coordinate inside the tile
+    const unsigned int fine_x = x % 8;
+    const unsigned int fine_y = y % 8;
+
+    const u8* line = source + coarse_y * info.stride;
+    const u8* tile = line + coarse_x * CalculateTileSize(info.format);
+    return LookupTexelInTile(tile, fine_x, fine_y, info, disable_alpha);
+}
+
+Math::Vec4<u8> LookupTexelInTile(const u8* source, unsigned int x, unsigned int y,
+                                 const TextureInfo& info, bool disable_alpha) {
+    DEBUG_ASSERT(x < 8);
+    DEBUG_ASSERT(y < 8);
+
+    using VideoCore::MortonInterleave;
 
     switch (info.format) {
     case Regs::TextureFormat::RGBA8: {
-        auto res = Color::DecodeRGBA8(source + VideoCore::GetMortonOffset(x, y, 4));
+        auto res = Color::DecodeRGBA8(source + MortonInterleave(x, y) * 4);
         return {res.r(), res.g(), res.b(), static_cast<u8>(disable_alpha ? 255 : res.a())};
     }
 
     case Regs::TextureFormat::RGB8: {
-        auto res = Color::DecodeRGB8(source + VideoCore::GetMortonOffset(x, y, 3));
+        auto res = Color::DecodeRGB8(source + MortonInterleave(x, y) * 3);
         return {res.r(), res.g(), res.b(), 255};
     }
 
     case Regs::TextureFormat::RGB5A1: {
-        auto res = Color::DecodeRGB5A1(source + VideoCore::GetMortonOffset(x, y, 2));
+        auto res = Color::DecodeRGB5A1(source + MortonInterleave(x, y) * 2);
         return {res.r(), res.g(), res.b(), static_cast<u8>(disable_alpha ? 255 : res.a())};
     }
 
     case Regs::TextureFormat::RGB565: {
-        auto res = Color::DecodeRGB565(source + VideoCore::GetMortonOffset(x, y, 2));
+        auto res = Color::DecodeRGB565(source + MortonInterleave(x, y) * 2);
         return {res.r(), res.g(), res.b(), 255};
     }
 
     case Regs::TextureFormat::RGBA4: {
-        auto res = Color::DecodeRGBA4(source + VideoCore::GetMortonOffset(x, y, 2));
+        auto res = Color::DecodeRGBA4(source + MortonInterleave(x, y) * 2);
         return {res.r(), res.g(), res.b(), static_cast<u8>(disable_alpha ? 255 : res.a())};
     }
 
     case Regs::TextureFormat::IA8: {
-        const u8* source_ptr = source + VideoCore::GetMortonOffset(x, y, 2);
+        const u8* source_ptr = source + MortonInterleave(x, y) * 2;
 
         if (disable_alpha) {
             // Show intensity as red, alpha as green
@@ -65,17 +117,17 @@ Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo&
     }
 
     case Regs::TextureFormat::RG8: {
-        auto res = Color::DecodeRG8(source + VideoCore::GetMortonOffset(x, y, 2));
+        auto res = Color::DecodeRG8(source + MortonInterleave(x, y) * 2);
         return {res.r(), res.g(), 0, 255};
     }
 
     case Regs::TextureFormat::I8: {
-        const u8* source_ptr = source + VideoCore::GetMortonOffset(x, y, 1);
+        const u8* source_ptr = source + MortonInterleave(x, y);
         return {*source_ptr, *source_ptr, *source_ptr, 255};
     }
 
     case Regs::TextureFormat::A8: {
-        const u8* source_ptr = source + VideoCore::GetMortonOffset(x, y, 1);
+        const u8* source_ptr = source + MortonInterleave(x, y);
 
         if (disable_alpha) {
             return {*source_ptr, *source_ptr, *source_ptr, 255};
@@ -85,7 +137,7 @@ Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo&
     }
 
     case Regs::TextureFormat::IA4: {
-        const u8* source_ptr = source + VideoCore::GetMortonOffset(x, y, 1);
+        const u8* source_ptr = source + MortonInterleave(x, y);
 
         u8 i = Color::Convert4To8(((*source_ptr) & 0xF0) >> 4);
         u8 a = Color::Convert4To8((*source_ptr) & 0xF);
@@ -99,7 +151,7 @@ Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo&
     }
 
     case Regs::TextureFormat::I4: {
-        u32 morton_offset = VideoCore::GetMortonOffset(x, y, 1);
+        u32 morton_offset = MortonInterleave(x, y);
         const u8* source_ptr = source + morton_offset / 2;
 
         u8 i = (morton_offset % 2) ? ((*source_ptr & 0xF0) >> 4) : (*source_ptr & 0xF);
@@ -109,7 +161,7 @@ Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo&
     }
 
     case Regs::TextureFormat::A4: {
-        u32 morton_offset = VideoCore::GetMortonOffset(x, y, 1);
+        u32 morton_offset = MortonInterleave(x, y);
         const u8* source_ptr = source + morton_offset / 2;
 
         u8 a = (morton_offset % 2) ? ((*source_ptr & 0xF0) >> 4) : (*source_ptr & 0xF);
@@ -127,15 +179,15 @@ Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo&
         bool has_alpha = (info.format == Regs::TextureFormat::ETC1A4);
 
         // ETC1 further subdivides each 8x8 tile into four 4x4 subtiles
-        const int subtile_width = 4;
+        constexpr unsigned int subtile_width = 4;
-        const int subtile_height = 4;
+        constexpr unsigned int subtile_height = 4;
 
-        int subtile_index = ((x / subtile_width) & 1) + 2 * ((y / subtile_height) & 1);
+        unsigned int subtile_index = (x / subtile_width) + 2 * (y / subtile_height);
-        unsigned subtile_bytes = has_alpha ? 2 : 1; // TODO: Name...
+        size_t subtile_size = has_alpha ? 16 : 8;
+
+        // TODO(yuriks): Use memcpy instead of reinterpret_cast
+        const u64* source_ptr = reinterpret_cast<const u64*>(source + subtile_index * subtile_size);
 
-        const u64* source_ptr = (const u64*)(source + coarse_x * subtile_bytes * 4 +
-                                             coarse_y * subtile_bytes * 4 * (info.width / 8) +
-                                             subtile_index * subtile_bytes * 8);
         u64 alpha = 0xFFFFFFFFFFFFFFFF;
         if (has_alpha) {
             alpha = *source_ptr;
@@ -262,7 +314,7 @@ TextureInfo TextureInfo::FromPicaRegister(const Regs::TextureConfig& config,
     info.width = config.width;
     info.height = config.height;
     info.format = format;
-    info.stride = Pica::Regs::NibblesPerPixel(info.format) * info.width / 2;
+    info.SetDefaultStride();
     return info;
 }
 

src/video_core/texture/texture_decode.h

@@ -11,21 +11,29 @@
 namespace Pica {
 namespace Texture {
 
+/// Returns the byte size of a 8*8 tile of the specified texture format.
+size_t CalculateTileSize(Pica::Regs::TextureFormat format);
+
 struct TextureInfo {
     PAddr physical_address;
-    int width;
+    unsigned int width;
-    int height;
+    unsigned int height;
-    int stride;
+    ptrdiff_t stride;
     Pica::Regs::TextureFormat format;
 
     static TextureInfo FromPicaRegister(const Pica::Regs::TextureConfig& config,
                                         const Pica::Regs::TextureFormat& format);
+
+    /// Calculates stride from format and width, assuming that the entire texture is contiguous.
+    void SetDefaultStride() {
+        stride = Pica::Texture::CalculateTileSize(format) * (width / 8);
+    }
 };
 
 /**
  * Lookup texel located at the given coordinates and return an RGBA vector of its color.
  * @param source Source pointer to read data from
- * @param s,t Texture coordinates to read from
+ * @param x,y Texture coordinates to read from
  * @param info TextureInfo object describing the texture setup
  * @param disable_alpha This is used for debug widgets which use this method to display textures
  * without providing a good way to visualize alpha by themselves. If true, this will return 255 for
@@ -33,8 +41,20 @@ struct TextureInfo {
  * channel.
  * @todo Eventually we should get rid of the disable_alpha parameter.
  */
-Math::Vec4<u8> LookupTexture(const u8* source, int s, int t, const TextureInfo& info,
+Math::Vec4<u8> LookupTexture(const u8* source, unsigned int x, unsigned int y,
-                             bool disable_alpha = false);
+                             const TextureInfo& info, bool disable_alpha = false);
+
+/**
+ * Looks up a texel from a single 8x8 texture tile.
+ *
+ * @param source Pointer to the beginning of the tile.
+ * @param x, y In-tile coordinates to read from. Must be < 8.
+ * @param info TextureInfo describing the texture format.
+ * @param disable_alpha Used for debugging. Sets the result alpha to 255 and either discards the
+ *                      real alpha or inserts it in an otherwise unused channel.
+ */
+Math::Vec4<u8> LookupTexelInTile(const u8* source, unsigned int x, unsigned int y,
+                                 const TextureInfo& info, bool disable_alpha);
 
 } // namespace Texture
 } // namespace Pica