Merge pull request #218 from neobrain/pica_debugger

Pica debugger improvements

4 files changed, 269 insertions, 28 deletions

diff --git a/src/video_core/command_processor.cpp b/src/video_core/command_processor.cpp
index 8a6ba2560..298b04c51 100644
--- a/src/video_core/command_processor.cpp
+++ b/src/video_core/command_processor.cpp
@@ -34,6 +34,9 @@ static inline void WritePicaReg(u32 id, u32 value, u32 mask) {
     u32 old_value = registers[id];
     registers[id] = (old_value & ~mask) | (value & mask);
 
+    if (g_debug_context)
+        g_debug_context->OnEvent(DebugContext::Event::CommandLoaded, reinterpret_cast<void*>(&id));
+
     DebugUtils::OnPicaRegWrite(id, registers[id]);
 
     switch(id) {
@@ -43,6 +46,9 @@ static inline void WritePicaReg(u32 id, u32 value, u32 mask) {
         {
             DebugUtils::DumpTevStageConfig(registers.GetTevStages());
 
+            if (g_debug_context)
+                g_debug_context->OnEvent(DebugContext::Event::IncomingPrimitiveBatch, nullptr);
+
             const auto& attribute_config = registers.vertex_attributes;
             const u8* const base_address = Memory::GetPointer(attribute_config.GetBaseAddress());
 
@@ -132,6 +138,10 @@ static inline void WritePicaReg(u32 id, u32 value, u32 mask) {
                 clipper_primitive_assembler.SubmitVertex(output, Clipper::ProcessTriangle);
             }
             geometry_dumper.Dump();
+
+            if (g_debug_context)
+                g_debug_context->OnEvent(DebugContext::Event::FinishedPrimitiveBatch, nullptr);
+
             break;
         }
 
@@ -229,6 +239,9 @@ static inline void WritePicaReg(u32 id, u32 value, u32 mask) {
         default:
             break;
     }
+
+    if (g_debug_context)
+        g_debug_context->OnEvent(DebugContext::Event::CommandProcessed, reinterpret_cast<void*>(&id));
 }
 
 static std::ptrdiff_t ExecuteCommandBlock(const u32* first_command_word) {
diff --git a/src/video_core/debug_utils/debug_utils.cpp b/src/video_core/debug_utils/debug_utils.cpp
index 8a5f11424..71b03f31c 100644
--- a/src/video_core/debug_utils/debug_utils.cpp
+++ b/src/video_core/debug_utils/debug_utils.cpp
@@ -3,6 +3,8 @@
 // Refer to the license.txt file included.
 
 #include <algorithm>
+#include <condition_variable>
+#include <list>
 #include <map>
 #include <fstream>
 #include <mutex>
@@ -12,14 +14,56 @@
 #include <png.h>
 #endif
 
+#include "common/log.h"
 #include "common/file_util.h"
 
+#include "video_core/math.h"
 #include "video_core/pica.h"
 
 #include "debug_utils.h"
 
 namespace Pica {
 
+void DebugContext::OnEvent(Event event, void* data) {
+    if (!breakpoints[event].enabled)
+        return;
+
+    {
+        std::unique_lock<std::mutex> lock(breakpoint_mutex);
+
+        // TODO: Should stop the CPU thread here once we multithread emulation.
+
+        active_breakpoint = event;
+        at_breakpoint = true;
+
+        // Tell all observers that we hit a breakpoint
+        for (auto& breakpoint_observer : breakpoint_observers) {
+            breakpoint_observer->OnPicaBreakPointHit(event, data);
+        }
+
+        // Wait until another thread tells us to Resume()
+        resume_from_breakpoint.wait(lock, [&]{ return !at_breakpoint; });
+    }
+}
+
+void DebugContext::Resume() {
+    {
+        std::unique_lock<std::mutex> lock(breakpoint_mutex);
+
+        // Tell all observers that we are about to resume
+        for (auto& breakpoint_observer : breakpoint_observers) {
+            breakpoint_observer->OnPicaResume();
+        }
+
+        // Resume the waiting thread (i.e. OnEvent())
+        at_breakpoint = false;
+    }
+
+    resume_from_breakpoint.notify_one();
+}
+
+std::shared_ptr<DebugContext> g_debug_context; // TODO: Get rid of this global
+
 namespace DebugUtils {
 
 void GeometryDumper::AddTriangle(Vertex& v0, Vertex& v1, Vertex& v2) {
@@ -312,6 +356,42 @@ std::unique_ptr<PicaTrace> FinishPicaTracing()
     return std::move(ret);
 }
 
+const Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo& info) {
+    _dbg_assert_(GPU, info.format == Pica::Regs::TextureFormat::RGB8);
+
+    // Cf. rasterizer code for an explanation of this algorithm.
+    int texel_index_within_tile = 0;
+    for (int block_size_index = 0; block_size_index < 3; ++block_size_index) {
+        int sub_tile_width = 1 << block_size_index;
+        int sub_tile_height = 1 << block_size_index;
+
+        int sub_tile_index = (x & sub_tile_width) << block_size_index;
+        sub_tile_index += 2 * ((y & sub_tile_height) << block_size_index);
+        texel_index_within_tile += sub_tile_index;
+    }
+
+    const int block_width = 8;
+    const int block_height = 8;
+
+    int coarse_x = (x / block_width) * block_width;
+    int coarse_y = (y / block_height) * block_height;
+
+    const u8* source_ptr = source + coarse_x * block_height * 3 + coarse_y * info.stride + texel_index_within_tile * 3;
+    return { source_ptr[2], source_ptr[1], source_ptr[0], 255 };
+}
+
+TextureInfo TextureInfo::FromPicaRegister(const Regs::TextureConfig& config,
+                                          const Regs::TextureFormat& format)
+{
+    TextureInfo info;
+    info.address = config.GetPhysicalAddress();
+    info.width = config.width;
+    info.height = config.height;
+    info.format = format;
+    info.stride = Pica::Regs::BytesPerPixel(info.format) * info.width;
+    return info;
+}
+
 void DumpTexture(const Pica::Regs::TextureConfig& texture_config, u8* data) {
     // NOTE: Permanently enabling this just trashes hard disks for no reason.
     //       Hence, this is currently disabled.
@@ -377,27 +457,15 @@ void DumpTexture(const Pica::Regs::TextureConfig& texture_config, u8* data) {
     buf = new u8[row_stride * texture_config.height];
     for (unsigned y = 0; y < texture_config.height; ++y) {
         for (unsigned x = 0; x < texture_config.width; ++x) {
-            // Cf. rasterizer code for an explanation of this algorithm.
-            int texel_index_within_tile = 0;
-            for (int block_size_index = 0; block_size_index < 3; ++block_size_index) {
-                int sub_tile_width = 1 << block_size_index;
-                int sub_tile_height = 1 << block_size_index;
-
-                int sub_tile_index = (x & sub_tile_width) << block_size_index;
-                sub_tile_index += 2 * ((y & sub_tile_height) << block_size_index);
-                texel_index_within_tile += sub_tile_index;
-            }
-
-            const int block_width = 8;
-            const int block_height = 8;
-
-            int coarse_x = (x / block_width) * block_width;
-            int coarse_y = (y / block_height) * block_height;
-
-            u8* source_ptr = (u8*)data + coarse_x * block_height * 3 + coarse_y * row_stride + texel_index_within_tile * 3;
-            buf[3 * x + y * row_stride    ] = source_ptr[2];
-            buf[3 * x + y * row_stride + 1] = source_ptr[1];
-            buf[3 * x + y * row_stride + 2] = source_ptr[0];
+            TextureInfo info;
+            info.width = texture_config.width;
+            info.height = texture_config.height;
+            info.stride = row_stride;
+            info.format = registers.texture0_format;
+            Math::Vec4<u8> texture_color = LookupTexture(data, x, y, info);
+            buf[3 * x + y * row_stride    ] = texture_color.r();
+            buf[3 * x + y * row_stride + 1] = texture_color.g();
+            buf[3 * x + y * row_stride + 2] = texture_color.b();
         }
     }
 
diff --git a/src/video_core/debug_utils/debug_utils.h b/src/video_core/debug_utils/debug_utils.h
index b1558cfae..51f14f12f 100644
--- a/src/video_core/debug_utils/debug_utils.h
+++ b/src/video_core/debug_utils/debug_utils.h
@@ -5,13 +5,147 @@
 #pragma once
 
 #include <array>
+#include <condition_variable>
+#include <list>
+#include <map>
 #include <memory>
+#include <mutex>
 #include <vector>
 
+#include "video_core/math.h"
 #include "video_core/pica.h"
 
 namespace Pica {
 
+class DebugContext {
+public:
+    enum class Event {
+        FirstEvent = 0,
+
+        CommandLoaded = FirstEvent,
+        CommandProcessed,
+        IncomingPrimitiveBatch,
+        FinishedPrimitiveBatch,
+
+        NumEvents
+    };
+
+    /**
+     * Inherit from this class to be notified of events registered to some debug context.
+     * Most importantly this is used for our debugger GUI.
+     *
+     * To implement event handling, override the OnPicaBreakPointHit and OnPicaResume methods.
+     * @warning All BreakPointObservers need to be on the same thread to guarantee thread-safe state access
+     * @todo Evaluate an alternative interface, in which there is only one managing observer and multiple child observers running (by design) on the same thread.
+     */
+    class BreakPointObserver {
+    public:
+        /// Constructs the object such that it observes events of the given DebugContext.
+        BreakPointObserver(std::shared_ptr<DebugContext> debug_context) : context_weak(debug_context) {
+            std::unique_lock<std::mutex> lock(debug_context->breakpoint_mutex);
+            debug_context->breakpoint_observers.push_back(this);
+        }
+
+        virtual ~BreakPointObserver() {
+            auto context = context_weak.lock();
+            if (context) {
+                std::unique_lock<std::mutex> lock(context->breakpoint_mutex);
+                context->breakpoint_observers.remove(this);
+
+                // If we are the last observer to be destroyed, tell the debugger context that
+                // it is free to continue. In particular, this is required for a proper Citra
+                // shutdown, when the emulation thread is waiting at a breakpoint.
+                if (context->breakpoint_observers.empty())
+                    context->Resume();
+            }
+        }
+
+        /**
+         * Action to perform when a breakpoint was reached.
+         * @param event Type of event which triggered the breakpoint
+         * @param data Optional data pointer (if unused, this is a nullptr)
+         * @note This function will perform nothing unless it is overridden in the child class.
+         */
+        virtual void OnPicaBreakPointHit(Event, void*) {
+        }
+
+        /**
+         * Action to perform when emulation is resumed from a breakpoint.
+         * @note This function will perform nothing unless it is overridden in the child class.
+         */
+        virtual void OnPicaResume() {
+        }
+
+    protected:
+        /**
+         * Weak context pointer. This need not be valid, so when requesting a shared_ptr via
+         * context_weak.lock(), always compare the result against nullptr.
+         */
+        std::weak_ptr<DebugContext> context_weak;
+    };
+
+    /**
+     * Simple structure defining a breakpoint state
+     */
+    struct BreakPoint {
+        bool enabled = false;
+    };
+
+    /**
+     * Static constructor used to create a shared_ptr of a DebugContext.
+     */
+    static std::shared_ptr<DebugContext> Construct() {
+        return std::shared_ptr<DebugContext>(new DebugContext);
+    }
+
+    /**
+     * Used by the emulation core when a given event has happened. If a breakpoint has been set
+     * for this event, OnEvent calls the event handlers of the registered breakpoint observers.
+     * The current thread then is halted until Resume() is called from another thread (or until
+     * emulation is stopped).
+     * @param event Event which has happened
+     * @param data Optional data pointer (pass nullptr if unused). Needs to remain valid until Resume() is called.
+     */
+    void OnEvent(Event event, void* data);
+
+    /**
+     * Resume from the current breakpoint.
+     * @warning Calling this from the same thread that OnEvent was called in will cause a deadlock. Calling from any other thread is safe.
+     */
+    void Resume();
+
+    /**
+     * Delete all set breakpoints and resume emulation.
+     */
+    void ClearBreakpoints() {
+        breakpoints.clear();
+        Resume();
+    }
+
+    // TODO: Evaluate if access to these members should be hidden behind a public interface.
+    std::map<Event, BreakPoint> breakpoints;
+    Event active_breakpoint;
+    bool at_breakpoint = false;
+
+private:
+    /**
+     * Private default constructor to make sure people always construct this through Construct()
+     * instead.
+     */
+    DebugContext() = default;
+
+    /// Mutex protecting current breakpoint state and the observer list.
+    std::mutex breakpoint_mutex;
+
+    /// Used by OnEvent to wait for resumption.
+    std::condition_variable resume_from_breakpoint;
+
+    /// List of registered observers
+    std::list<BreakPointObserver*> breakpoint_observers;
+};
+
+extern std::shared_ptr<DebugContext> g_debug_context; // TODO: Get rid of this global
+
 namespace DebugUtils {
 
 // Simple utility class for dumping geometry data to an OBJ file
@@ -57,6 +191,18 @@ bool IsPicaTracing();
 void OnPicaRegWrite(u32 id, u32 value);
 std::unique_ptr<PicaTrace> FinishPicaTracing();
 
+struct TextureInfo {
+    unsigned int address;
+    int width;
+    int height;
+    int stride;
+    Pica::Regs::TextureFormat format;
+
+    static TextureInfo FromPicaRegister(const Pica::Regs::TextureConfig& config,
+                                        const Pica::Regs::TextureFormat& format);
+};
+
+const Math::Vec4<u8> LookupTexture(const u8* source, int x, int y, const TextureInfo& info);
 void DumpTexture(const Pica::Regs::TextureConfig& texture_config, u8* data);
 
 void DumpTevStageConfig(const std::array<Pica::Regs::TevStageConfig,6>& stages);
diff --git a/src/video_core/pica.h b/src/video_core/pica.h
index 5fe15a218..e7ca38978 100644
--- a/src/video_core/pica.h
+++ b/src/video_core/pica.h
@@ -109,7 +109,7 @@ struct Regs {
 
         u32 address;
 
-        u32 GetPhysicalAddress() {
+        u32 GetPhysicalAddress() const {
             return DecodeAddressRegister(address) - Memory::FCRAM_PADDR + Memory::HEAP_GSP_VADDR;
         }
 
@@ -130,7 +130,26 @@ struct Regs {
         // Seems like they are luminance formats and compressed textures.
     };
 
-    BitField<0, 1, u32> texturing_enable;
+    static unsigned BytesPerPixel(TextureFormat format) {
+        switch (format) {
+        case TextureFormat::RGBA8:
+            return 4;
+
+        case TextureFormat::RGB8:
+            return 3;
+
+        case TextureFormat::RGBA5551:
+        case TextureFormat::RGB565:
+        case TextureFormat::RGBA4:
+            return 2;
+
+        default:
+            // placeholder for yet unknown formats
+            return 1;
+        }
+    }
+
+    BitField< 0, 1, u32> texturing_enable;
     TextureConfig texture0;
     INSERT_PADDING_WORDS(0x8);
     BitField<0, 4, TextureFormat> texture0_format;
@@ -517,10 +536,6 @@ struct Regs {
     static std::string GetCommandName(int index) {
         std::map<u32, std::string> map;
 
-        // TODO: MSVC does not support using offsetof() on non-static data members even though this
-        //       is technically allowed since C++11. Hence, this functionality is disabled until
-        //       MSVC properly supports it.
-        #ifndef _MSC_VER
         Regs regs;
         #define ADD_FIELD(name)                                                                               \
             do {                                                                                              \
@@ -557,7 +572,6 @@ struct Regs {
         ADD_FIELD(vs_swizzle_patterns);
 
         #undef ADD_FIELD
-        #endif // _MSC_VER
 
         // Return empty string if no match is found
         return map[index];