From 3445ab9304081f30e28a9f91ba8135bdbde59aca Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 14 Jul 2026 02:37:41 -0400 Subject: [PATCH] [Refactor] (MG_Backend/DirectVulkan, trace-replay): extract dump-image capture out of MobileGL backend Moves snapshot capture entirely into the apitrace retrace layer (glReadPixels + PNG encode). Drops the MOBILEGL_PRESENT_DUMP_PATH / MOBILEGL_PRESENT_STATS / MOBILEGL_PRESENT_DUMP_CALL / MOBILEGL_PRESENT_CURRENT_CALL / MOBILEGL_TRACE_CURRENT_CALL_OVERRIDE plumbing from Config, ConfigLoader, VulkanRenderer (GetPresentedDumpPixel/WritePresentedDumpPpm + present-stats readback), the EGL/GLX/Android ws shims, and the Android trace_replay_core PPM reader. DirectVulkan ReadPixels on the default framebuffer now remaps raw swapchain pixels (top-left origin, preTransform-rotated) to GL orientation (bottom-left origin) so the retrace snapshot matches the golden; SwapchainObject also resizes the default-FBO stencil attachment to the swapchain extent to fix GL_INVALID_FRAMEBUFFER_OPERATION under the glReadPixels completeness check. --- 3rdparty/apitrace | 2 +- MobileGL/Config.h | 6 - MobileGL/ConfigLoader.cpp | 2 - .../DirectVulkan/Renderer/SwapchainObject.cpp | 24 ++ .../DirectVulkan/Renderer/VulkanRenderer.cpp | 270 ++++-------------- android-plugin/TRACE_REPLAY.md | 1 + .../src/trace/cpp/apitrace_glproc_android.cpp | 25 -- .../src/trace/cpp/apitrace_glws_android.cpp | 16 +- .../app/src/trace/cpp/trace_replay_core.cpp | 190 ++---------- tools/trace_replay/apitrace_glws_egl.cpp | 9 +- .../skills/mismatch-retrace-debugging.md | 4 +- 11 files changed, 105 insertions(+), 444 deletions(-) diff --git a/3rdparty/apitrace b/3rdparty/apitrace index efee00b9..10935bb5 160000 --- a/3rdparty/apitrace +++ b/3rdparty/apitrace @@ -1 +1 @@ -Subproject commit efee00b9999ef07ff955fbf2546cc242074bc037 +Subproject commit 10935bb5e466c84ff81141badcbe50b5745ea200 diff --git a/MobileGL/Config.h b/MobileGL/Config.h index 5933c14a..c0f32c52 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -27,8 +27,6 @@ namespace MobileGL::MG_Config { // // Env variables intentionally NOT mirrored here (kept as live std::getenv at their // call sites): - // - MOBILEGL_PRESENT_DUMP_CALL / MOBILEGL_PRESENT_CURRENT_CALL: the retrace harness - // mutates them at runtime via setenv, so a one-shot snapshot would go stale. // - DISPLAY: X11 session variable, not MobileGL configuration. // - MOBILEGL_LOG_FILE_PATH: log-file init runs before MG_ConfigLoader::Init // (see MG_Util/Debug/Log.cpp). @@ -46,10 +44,6 @@ namespace MobileGL::MG_Config { // MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER: avoid mipmap min filters in samplers, // resolves certain rendering bugs on ANGLE + llvmpipe. Bool AvoidSamplerMipmapMinFilter = false; - // MOBILEGL_PRESENT_DUMP_PATH: directory for present frame dumps (DirectVulkan). - String PresentDumpPath; - // MOBILEGL_PRESENT_STATS: log present pixel statistics (DirectVulkan backend). - Bool PresentStats = false; // MOBILEGL_TRACE_SKIP_AUTODESTROY: skip teardown in the ELF destructor (Init.cpp). Bool TraceSkipAutodestroy = false; }; diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index f2de7fe7..a7b0b515 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -116,8 +116,6 @@ namespace MobileGL::MG_ConfigLoader { features.MagmaFramesInFlight = QueryEnvUint32("MOBILEGL_MAGMA_FRAMESINFLIGHT", 3, 1, 64); features.AvoidSamplerMipmapMinFilter = QueryEnvFlag("MOBILEGL_AVOID_SAMPLER_MIPMAP_MIN_FILTER"); - QueryEnvVariable("MOBILEGL_PRESENT_DUMP_PATH", features.PresentDumpPath, ""); - features.PresentStats = QueryEnvFlag("MOBILEGL_PRESENT_STATS"); features.TraceSkipAutodestroy = QueryEnvFlag("MOBILEGL_TRACE_SKIP_AUTODESTROY"); } diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/SwapchainObject.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/SwapchainObject.cpp index 5da2a293..e58c079b 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/SwapchainObject.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/SwapchainObject.cpp @@ -298,6 +298,30 @@ namespace MobileGL::MG_Backend::DirectVulkan { {extentWidth, extentHeight, 1}, defaultAttachmentByteSize}); // TODO: 4 is format size + // The default FBO's stencil attachment must track the swapchain extent: + // FramebufferObject::CheckCompleteness requires every valid attachment + // to share the same dimensions, and Init.cpp leaves a 512x512 placeholder. + // Without this the retrace-layer glReadPixels snapshot fails with + // GL_INVALID_FRAMEBUFFER_OPERATION on DirectVulkan. + TextureInternalFormat stencilFormat = TextureInternalFormat::Depth24Stencil8; + switch (m_depthStencilFormat) { + case VK_FORMAT_D32_SFLOAT_S8_UINT: + stencilFormat = TextureInternalFormat::Depth32FStencil8; + break; + case VK_FORMAT_D24_UNORM_S8_UINT: + stencilFormat = TextureInternalFormat::Depth24Stencil8; + break; + default: + // No stencil plane; mirror the depth format for consistency. + stencilFormat = depthFormat; + break; + } + auto* stencilTex = static_cast(defaultFBOInfo->stencilAttachment.get()); + stencilTex->SetInternalFormat(stencilFormat); + stencilTex->AllocateStorage(TextureUploadTarget::Texture2D, 0, { + {extentWidth, extentHeight, 1}, + defaultAttachmentByteSize}); // TODO: 4 is format size + } void SwapchainObject::CreateDepthStencilResources(VkDevice device, VkPhysicalDevice physicalDevice) { diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp index 9afb16ba..e17ed5fe 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VulkanRenderer.cpp @@ -21,7 +21,6 @@ #include "MG_Util/Converters/MGToVk/TextureEnumConverter.h" #include "MG_Util/Metrics/TextureMetrics.h" #include -#include #include #include #include @@ -639,24 +638,6 @@ namespace MobileGL::MG_Backend::DirectVulkan { return attributeMask; } - static const char* PresentDumpPath() { - const String& path = MG_Config::Features.PresentDumpPath; - return path.empty() ? nullptr : path.c_str(); - } - - static Bool PresentDumpMatchesTargetCall() { - // MOBILEGL_PRESENT_DUMP_CALL / MOBILEGL_PRESENT_CURRENT_CALL stay live - // getenv on purpose: the retrace harness mutates them at runtime via - // setenv to select which eglSwapBuffers call gets dumped, so they must - // not be snapshotted into MG_Config::Features at init time. - const char* target = std::getenv("MOBILEGL_PRESENT_DUMP_CALL"); - if (target == nullptr || target[0] == '\0') { - return true; - } - const char* current = std::getenv("MOBILEGL_PRESENT_CURRENT_CALL"); - return current != nullptr && std::strcmp(target, current) == 0; - } - static Bool TryGetCurrentVertexAttributeFormat(GLenum glType, VkFormat& outFormat) { switch (glType) { case GL_FLOAT: @@ -1516,10 +1497,6 @@ void main() { } } - static Bool PresentStatsEnabled() { - return MG_Config::Features.PresentStats; - } - static Bool IsBgraVkFormat(VkFormat format) { switch (format) { case VK_FORMAT_B8G8R8A8_UNORM: @@ -1533,65 +1510,46 @@ void main() { } } - static VkExtent2D GetPresentedDumpExtent(VkExtent2D rawExtent, - VkSurfaceTransformFlagBitsKHR preTransform) { + // Remap raw swapchain pixels (top-left origin, preTransform-rotated) into + // GL-oriented pixels (bottom-left origin) for the retrace snapshot path. + // Mirrors the removed GetPresentedDumpPixel mapping plus the Y-origin flip + // apitrace's flipped=true Image expects. Only identity/180 share the + // swapchain extent with the default framebuffer; 90/270 swap extents and + // are not handled here. + static Bool RemapDefaultFboReadbackToGLOrientation(const Uint8* rawPixels, + VkExtent2D rawExtent, + VkSurfaceTransformFlagBitsKHR preTransform, + Uint8* outPixels) { if (IsQuarterTurnPreTransform(preTransform)) { - return {rawExtent.height, rawExtent.width}; - } - return rawExtent; - } - - static const Uint8* GetPresentedDumpPixel(const Uint8* rawPixels, - VkExtent2D rawExtent, - VkSurfaceTransformFlagBitsKHR preTransform, - Uint32 presentedX, - Uint32 presentedY) { - Uint32 rawX = presentedX; - Uint32 rawY = presentedY; - switch (preTransform) { - case VK_SURFACE_TRANSFORM_ROTATE_90_BIT_KHR: - rawX = rawExtent.width - 1 - presentedY; - rawY = presentedX; - break; - case VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR: - rawX = rawExtent.width - 1 - presentedX; - rawY = rawExtent.height - 1 - presentedY; - break; - case VK_SURFACE_TRANSFORM_ROTATE_270_BIT_KHR: - rawX = presentedY; - rawY = rawExtent.height - 1 - presentedX; - break; - default: - break; - } - return rawPixels + (static_cast(rawY) * rawExtent.width + rawX) * 4; - } - - static Bool WritePresentedDumpPpm(const char* path, - const Uint8* rawPixels, - VkExtent2D rawExtent, - VkFormat rawFormat, - VkSurfaceTransformFlagBitsKHR preTransform) { - FILE* dump = std::fopen(path, "wb"); - if (dump == nullptr) { return false; } - - const VkExtent2D presentedExtent = GetPresentedDumpExtent(rawExtent, preTransform); - const Bool presentIsBgra = IsBgraVkFormat(rawFormat); - std::fprintf(dump, "P6\n%u %u\n255\n", presentedExtent.width, presentedExtent.height); - for (Uint32 y = 0; y < presentedExtent.height; ++y) { - for (Uint32 x = 0; x < presentedExtent.width; ++x) { - const Uint8* p = GetPresentedDumpPixel(rawPixels, rawExtent, preTransform, x, y); - const Uint8 rgb[3] = { - presentIsBgra ? p[2] : p[0], - p[1], - presentIsBgra ? p[0] : p[2], - }; - std::fwrite(rgb, 1, sizeof(rgb), dump); + const Uint32 w = rawExtent.width; + const Uint32 h = rawExtent.height; + if (w == 0 || h == 0) { + return false; + } + for (Uint32 outY = 0; outY < h; ++outY) { + const Uint32 displayY = h - 1 - outY; // GL bottom-origin -> display top-origin + for (Uint32 outX = 0; outX < w; ++outX) { + const Uint32 displayX = outX; + Uint32 rawX = displayX; + Uint32 rawY = displayY; + switch (preTransform) { + case VK_SURFACE_TRANSFORM_ROTATE_180_BIT_KHR: + rawX = w - 1 - displayX; + rawY = h - 1 - displayY; + break; + default: + break; + } + const Uint8* src = rawPixels + (static_cast(rawY) * w + rawX) * 4; + Uint8* dst = outPixels + (static_cast(outY) * w + outX) * 4; + dst[0] = src[0]; + dst[1] = src[1]; + dst[2] = src[2]; + dst[3] = src[3]; } } - std::fclose(dump); return true; } @@ -4804,16 +4762,6 @@ void main() { const VkImageLayout srcOriginalLayout = readIsDefaultFbo ? m_swapchainObject.GetImageLayout(m_imageIndexAcquired) : *srcBinding.trackedLayout; - if (PresentStatsEnabled()) { - std::fprintf(stderr, - "MOBILEGL_READPIXELS_BEGIN defaultFbo=%s x=%d y=%d width=%d height=%d srcLayout=%d recording=%s recorded=%s imageIndex=%u\n", - readIsDefaultFbo ? "true" : "false", - x, y, width, height, - static_cast(srcOriginalLayout), - frame.isCommandRecording ? "true" : "false", - frame.hasCommandBufferRecorded ? "true" : "false", - m_imageIndexAcquired); - } if (srcOriginalLayout == VK_IMAGE_LAYOUT_UNDEFINED) { MGLOG_E("DirectVulkan::ReadPixels skipped: source image layout is undefined"); return; @@ -4890,24 +4838,24 @@ void main() { MGLOG_E("DirectVulkan::ReadPixels skipped: failed to map readback buffer"); return; } - if (PresentStatsEnabled()) { - const SizeT pixelCount = static_cast(width) * static_cast(height); - SizeT nonBlack = 0; - SizeT nonTransparent = 0; - for (SizeT i = 0; i < pixelCount; ++i) { - const Uint8* p = mapped + i * 4; - if (p[0] != 0 || p[1] != 0 || p[2] != 0) { - ++nonBlack; - } - if (p[3] != 0) { - ++nonTransparent; + const VkFormat srcFormat = readIsDefaultFbo ? m_swapchainObject.GetSurfaceFormat().format : VK_FORMAT_R8G8B8A8_UNORM; + if (readIsDefaultFbo) { + const VkExtent2D swapchainExtent = m_swapchainObject.GetExtent(); + const VkSurfaceTransformFlagBitsKHR preTransform = m_swapchainObject.GetPreTransform(); + if (static_cast(width) == swapchainExtent.width && + static_cast(height) == swapchainExtent.height) { + Vector remapped(static_cast(width) * static_cast(height) * 4); + if (RemapDefaultFboReadbackToGLOrientation(mapped, swapchainExtent, preTransform, + remapped.data())) { + PackReadbackToClientOrPbo(remapped.data(), srcFormat, width, height, format, type, pixels); + return; } } - std::fprintf(stderr, - "MOBILEGL_READPIXELS_STATS nonBlack=%zu/%zu alpha=%zu/%zu\n", - nonBlack, pixelCount, nonTransparent, pixelCount); + MGLOG_W("DirectVulkan::ReadPixels: default-FBO remap skipped (w=%d h=%d swapchain=%ux%u preTransform=%d); " + "falling back to raw readback", + width, height, swapchainExtent.width, swapchainExtent.height, + static_cast(preTransform)); } - const VkFormat srcFormat = readIsDefaultFbo ? m_swapchainObject.GetSurfaceFormat().format : VK_FORMAT_R8G8B8A8_UNORM; PackReadbackToClientOrPbo(mapped, srcFormat, width, height, format, type, pixels); } @@ -5918,80 +5866,6 @@ void main() { if (activeRenderPass) VkRenderPassManager::EndRenderPass(frame.commandBuffer); - VkBufferObject presentStatsReadback; - VkDeviceSize presentStatsReadbackSize = 0; - const VkExtent2D presentStatsExtent = m_swapchainObject.GetExtent(); - const char* presentDumpPath = PresentDumpPath(); - const Bool shouldDumpPresent = presentDumpPath != nullptr && PresentDumpMatchesTargetCall(); - const Bool wantPresentStats = (PresentStatsEnabled() || shouldDumpPresent) && - presentStatsExtent.width > 0 && presentStatsExtent.height > 0; - if (wantPresentStats && !frame.isCommandRecording) { - // A mid-frame flush may have closed the frame's recording; the - // stats copy needs an open command buffer. - m_frameContext.BeginCommandRecording(); - } - const Bool collectPresentStats = wantPresentStats && frame.isCommandRecording; - if (PresentStatsEnabled() && presentDumpPath != nullptr) { - // Live getenv on purpose (not MG_Config::Features): the retrace - // harness mutates these two variables at runtime via setenv. - const char* targetCall = std::getenv("MOBILEGL_PRESENT_DUMP_CALL"); - const char* currentCall = std::getenv("MOBILEGL_PRESENT_CURRENT_CALL"); - std::fprintf(stderr, - "MOBILEGL_PRESENT_DUMP_GATE target=%s current=%s shouldDump=%s recording=%s size=%ux%u path=%s\n", - targetCall != nullptr ? targetCall : "", - currentCall != nullptr ? currentCall : "", - shouldDumpPresent ? "true" : "false", - frame.isCommandRecording ? "true" : "false", - presentStatsExtent.width, presentStatsExtent.height, - presentDumpPath); - } - if (collectPresentStats) { - presentStatsReadbackSize = static_cast(presentStatsExtent.width) * - static_cast(presentStatsExtent.height) * 4; - const Bool readbackCreated = presentStatsReadback.Create({ - .allocator = m_allocator, - .size = presentStatsReadbackSize, - .usage = VK_BUFFER_USAGE_TRANSFER_DST_BIT, - .memoryUsage = VMA_MEMORY_USAGE_AUTO, - .allocationFlags = VMA_ALLOCATION_CREATE_HOST_ACCESS_RANDOM_BIT, - }); - MOBILEGL_ASSERT(readbackCreated, "Present stats: failed to create readback buffer"); - - VkImageLayout statsOriginalLayout = m_swapchainObject.GetImageLayout(m_imageIndexAcquired); - Bool toTransferSrc = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, m_swapchainObject.GetImage(m_imageIndexAcquired), - statsOriginalLayout, VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT, VK_PIPELINE_STAGE_TRANSFER_BIT, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT, - VK_ACCESS_TRANSFER_READ_BIT, VK_IMAGE_ASPECT_COLOR_BIT); - MOBILEGL_ASSERT(toTransferSrc, "Present stats: failed to transition swapchain for readback"); - - VkBufferImageCopy copyRegion{}; - copyRegion.bufferOffset = 0; - copyRegion.bufferRowLength = 0; - copyRegion.bufferImageHeight = 0; - copyRegion.imageSubresource.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; - copyRegion.imageSubresource.mipLevel = 0; - copyRegion.imageSubresource.baseArrayLayer = 0; - copyRegion.imageSubresource.layerCount = 1; - copyRegion.imageOffset = {0, 0, 0}; - copyRegion.imageExtent = {presentStatsExtent.width, presentStatsExtent.height, 1}; - vkCmdCopyImageToBuffer(frame.commandBuffer, - m_swapchainObject.GetImage(m_imageIndexAcquired), - VK_IMAGE_LAYOUT_TRANSFER_SRC_OPTIMAL, - presentStatsReadback.GetHandle(), - 1, - ©Region); - - Bool restoreLayout = VkTextureManager::TransitionImageLayout( - frame.commandBuffer, m_swapchainObject.GetImage(m_imageIndexAcquired), - statsOriginalLayout, m_swapchainObject.GetImageLayout(m_imageIndexAcquired), - VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT, - VK_ACCESS_TRANSFER_READ_BIT, - VK_ACCESS_COLOR_ATTACHMENT_WRITE_BIT | VK_ACCESS_TRANSFER_WRITE_BIT, - VK_IMAGE_ASPECT_COLOR_BIT); - MOBILEGL_ASSERT(restoreLayout, "Present stats: failed to restore swapchain layout after readback"); - } if (frame.isCommandRecording) { m_frameContext.EndCommandRecording(); frame.hasCommandBufferRecorded = true; @@ -6008,50 +5882,6 @@ void main() { VK_VERIFY(vkQueueSubmit(m_graphicsQueue, 1, &submitPacket.submitInfo, frame.imageInFlightFence)); RegisterSubmit(frame.imageInFlightFence, /*pooledFence=*/false); frame.lastSubmitIndex = m_submitCounter; - if (collectPresentStats) { - VK_VERIFY(vkWaitForFences(m_device, 1, &frame.imageInFlightFence, VK_TRUE, UINT64_MAX), - "Present stats, vkWaitForFences"); - OnSubmitsCompletedUpTo(frame.lastSubmitIndex); - const auto* pixels = static_cast(presentStatsReadback.Map()); - MOBILEGL_ASSERT(pixels != nullptr, "Present stats: failed to map readback buffer"); - SizeT nonBlack = 0; - SizeT nonTransparent = 0; - SizeT colored = 0; - const SizeT pixelCount = static_cast(presentStatsExtent.width) * - static_cast(presentStatsExtent.height); - for (SizeT i = 0; i < pixelCount; ++i) { - const Uint8* p = pixels + i * 4; - if (p[0] != 0 || p[1] != 0 || p[2] != 0) { - ++nonBlack; - } - const Uint8 minRgb = std::min(p[0], std::min(p[1], p[2])); - const Uint8 maxRgb = std::max(p[0], std::max(p[1], p[2])); - if (maxRgb - minRgb > 24) { - ++colored; - } - if (p[3] != 0) { - ++nonTransparent; - } - } - if (PresentStatsEnabled()) { - std::fprintf(stderr, - "MOBILEGL_PRESENT_STATS nonBlack=%zu/%zu colored=%zu/%zu alpha=%zu/%zu size=%ux%u\n", - nonBlack, pixelCount, colored, pixelCount, nonTransparent, pixelCount, - presentStatsExtent.width, presentStatsExtent.height); - } - if (shouldDumpPresent) { - const VkFormat presentFormat = m_swapchainObject.GetSurfaceFormat().format; - if (WritePresentedDumpPpm(presentDumpPath, pixels, presentStatsExtent, presentFormat, - m_swapchainObject.GetPreTransform())) { - if (PresentStatsEnabled()) { - std::fprintf(stderr, "MOBILEGL_PRESENT_DUMP_WRITTEN path=%s bytes=%zu\n", - presentDumpPath, static_cast(pixelCount * 3)); - } - } else if (PresentStatsEnabled()) { - std::fprintf(stderr, "MOBILEGL_PRESENT_DUMP_FAILED path=%s\n", presentDumpPath); - } - } - } frame.isCommandRecording = false; frame.hasCommandBufferRecorded = false; m_swapchainObject.SetImageLayout(m_imageIndexAcquired, VK_IMAGE_LAYOUT_PRESENT_SRC_KHR); diff --git a/android-plugin/TRACE_REPLAY.md b/android-plugin/TRACE_REPLAY.md index d0df5356..a40c1178 100644 --- a/android-plugin/TRACE_REPLAY.md +++ b/android-plugin/TRACE_REPLAY.md @@ -46,6 +46,7 @@ Implementation notes: - The trace APK builds independently from FCL and can be launched with `adb shell am start`. - The native runner validates inputs, sets `MOBILEGL_BACKEND_TYPE`, loads `libMobileGL.so`, runs apitrace GL retrace, writes `actual.png`, and writes `result.json`. +- Snapshot capture stays entirely in the retrace layer: apitrace selects the target call, obtains drawable/read-buffer state, calls MobileGL's public `glReadPixels`, and encodes the returned pixels to PNG. MobileGL has no trace-call or output-path hooks. - The runner uses a MobileGL-backed EGL window-system shim. GLX calls in PC traces are consumed by apitrace's GLX retrace frontend and mapped onto this EGL shim; the Android runner does not require or call a MobileGL GLX implementation. - `DirectGLES` and `DirectVulkan` replay on the Activity `SurfaceView` by default. DirectGLES can still use the old offscreen EGL pbuffer path by passing `use_pbuffer=true`. - Golden comparison is implemented in native C++ with libpng RGBA decode and SSIM validation. The Java Activity only passes arguments and displays the native result, so the replay/compare core is not tied to Android UI or Bitmap APIs and can be ported to Linux. diff --git a/android-plugin/app/src/trace/cpp/apitrace_glproc_android.cpp b/android-plugin/app/src/trace/cpp/apitrace_glproc_android.cpp index 784da911..2f7c6e5d 100644 --- a/android-plugin/app/src/trace/cpp/apitrace_glproc_android.cpp +++ b/android-plugin/app/src/trace/cpp/apitrace_glproc_android.cpp @@ -1,8 +1,6 @@ #include "glproc.hpp" #include -#include -#include #include #include @@ -56,29 +54,6 @@ void MobileGLTraceReadPixels(GLint x, GLint y, GLsizei width, GLsizei height, GL if (gRealGlReadPixels != nullptr) { gRealGlReadPixels(x, y, width, height, format, type, pixels); } - const char *stats = getenv("MOBILEGL_PRESENT_STATS"); - if (stats == nullptr || stats[0] == '\0' || strcmp(stats, "0") == 0 || - pixels == nullptr || width <= 0 || height <= 0 || - format != GL_RGBA || type != GL_UNSIGNED_BYTE) { - return; - } - const auto *bytes = static_cast(pixels); - const size_t pixelCount = static_cast(width) * static_cast(height); - size_t nonBlack = 0; - size_t nonTransparent = 0; - for (size_t i = 0; i < pixelCount; ++i) { - const unsigned char *p = bytes + i * 4; - if (p[0] != 0 || p[1] != 0 || p[2] != 0) { - ++nonBlack; - } - if (p[3] != 0) { - ++nonTransparent; - } - } - fprintf(stderr, - "MOBILEGL_GLPROC_READPIXELS x=%d y=%d width=%d height=%d nonBlack=%zu/%zu alpha=%zu/%zu real=%p\n", - x, y, width, height, nonBlack, pixelCount, nonTransparent, pixelCount, - reinterpret_cast(gRealGlReadPixels)); } void MobileGLTraceGetIntegerv(GLenum pname, GLint *data) { diff --git a/android-plugin/app/src/trace/cpp/apitrace_glws_android.cpp b/android-plugin/app/src/trace/cpp/apitrace_glws_android.cpp index 34666b1a..625e18ef 100644 --- a/android-plugin/app/src/trace/cpp/apitrace_glws_android.cpp +++ b/android-plugin/app/src/trace/cpp/apitrace_glws_android.cpp @@ -183,22 +183,8 @@ public: void swapBuffers() override { if (surface != EGL_NO_SURFACE) { char callNo[32]; - const char *overrideCallNo = getenv("MOBILEGL_TRACE_CURRENT_CALL_OVERRIDE"); - if (overrideCallNo != nullptr && overrideCallNo[0] != '\0') { - snprintf(callNo, sizeof(callNo), "%s", overrideCallNo); - } else { - snprintf(callNo, sizeof(callNo), "%u", retrace::callNo); - } - const char *targetCall = getenv("MOBILEGL_PRESENT_DUMP_CALL"); - const char *presentStats = getenv("MOBILEGL_PRESENT_STATS"); - if (targetCall != nullptr && targetCall[0] != '\0' && - presentStats != nullptr && presentStats[0] != '\0' && strcmp(presentStats, "0") != 0) { - std::cerr << "MOBILEGL_TRACE_SWAP call=" << callNo - << " target=" << targetCall << "\n"; - } - setenv("MOBILEGL_PRESENT_CURRENT_CALL", callNo, 1); + snprintf(callNo, sizeof(callNo), "%u", retrace::callNo); gEgl.swapBuffers(gDisplay, surface); - unsetenv("MOBILEGL_PRESENT_CURRENT_CALL"); HoldAfterTargetPresent(callNo); } } diff --git a/android-plugin/app/src/trace/cpp/trace_replay_core.cpp b/android-plugin/app/src/trace/cpp/trace_replay_core.cpp index ed3bd464..845d9961 100644 --- a/android-plugin/app/src/trace/cpp/trace_replay_core.cpp +++ b/android-plugin/app/src/trace/cpp/trace_replay_core.cpp @@ -3,7 +3,6 @@ #include #include "apitrace_exit.hpp" #include "png.h" -#include "trace_parser.hpp" #include #include @@ -11,12 +10,10 @@ #include #include #include -#include #include #include #include #include -#include #include #include #include @@ -357,106 +354,15 @@ void ForceOpaqueAlpha(RgbaImage& image) { } } -bool ReadPpmRgbAsRgba(const std::string& path, RgbaImage& image, std::string& error) { - std::ifstream file(path, std::ios::binary); - if (!file) { - error = "failed to open PPM: " + path; - return false; - } - - auto readToken = [&file]() -> std::string { - std::string token; - char ch = 0; - while (file.get(ch)) { - if (ch == '#') { - file.ignore(std::numeric_limits::max(), '\n'); - continue; - } - if (!std::isspace(static_cast(ch))) { - token.push_back(ch); - break; - } - } - while (file.get(ch)) { - if (std::isspace(static_cast(ch))) { - break; - } - token.push_back(ch); - } - return token; - }; - - const std::string magic = readToken(); - const std::string widthToken = readToken(); - const std::string heightToken = readToken(); - const std::string maxToken = readToken(); - if (magic != "P6" || widthToken.empty() || heightToken.empty() || maxToken != "255") { - error = "unsupported PPM header: " + path; - return false; - } - - image.width = std::stoi(widthToken); - image.height = std::stoi(heightToken); - if (image.width <= 0 || image.height <= 0) { - error = "PPM has invalid dimensions: " + path; - return false; - } - - std::vector rgb(static_cast(image.width) * image.height * 3); - file.read(reinterpret_cast(rgb.data()), static_cast(rgb.size())); - if (file.gcount() != static_cast(rgb.size())) { - error = "PPM payload is truncated: " + path; - return false; - } - - image.pixels.resize(static_cast(image.width) * image.height * 4); - for (std::size_t i = 0, j = 0; i < rgb.size(); i += 3, j += 4) { - image.pixels[j + 0] = rgb[i + 0]; - image.pixels[j + 1] = rgb[i + 1]; - image.pixels[j + 2] = rgb[i + 2]; - image.pixels[j + 3] = 0xff; - } - return true; -} - -std::string SnapshotPathForCall(const Request& request, bool usePresentDump) { +std::string SnapshotPathForCall(const Request& request) { char call[16]; - snprintf(call, sizeof(call), "%010lld", usePresentDump ? request.targetCall + 1 : request.targetCall); + snprintf(call, sizeof(call), "%010lld", request.targetCall); return request.outputDir + "/actual." + call + ".png"; } -bool TargetCallSwapsRenderTarget(const Request& request, bool& swapsRenderTarget, std::string& error) { - trace::Parser parser; - if (!parser.open(request.tracePath.c_str())) { - error = "failed to open trace for target call inspection"; - return false; - } - - trace::Call* call = nullptr; - while ((call = parser.parse_call()) != nullptr) { - const long long callNo = static_cast(call->no); - if (callNo == request.targetCall) { - swapsRenderTarget = (call->flags & trace::CALL_FLAG_SWAP_RENDERTARGET) != 0; - delete call; - return true; - } - if (callNo > request.targetCall) { - delete call; - break; - } - delete call; - } - - std::ostringstream message; - message << "target_call " << request.targetCall << " was not found in trace"; - error = message.str(); - return false; -} - -int RunRetraceMain(const Request& request, bool usePresentDump) { +int RunRetraceMain(const Request& request) { std::string prefix = request.outputDir + "/actual."; - const long long snapshotCall = usePresentDump ? request.targetCall + 1 : request.targetCall; - std::string callSet = std::to_string(snapshotCall); + std::string callSet = std::to_string(request.targetCall); std::string arg0 = "mobilegl-glretrace"; std::string argBenchmark = "-b"; @@ -483,12 +389,12 @@ int RunRetraceMain(const Request& request, bool usePresentDump) { return MOBILEGL_APITRACE_RETRACE_MAIN(10, argv); } -bool RunRetrace(const Request& request, bool usePresentDump, Result& result) { +bool RunRetrace(const Request& request, Result& result) { int status = 0; ConfigureHoldEnv(request); try { ScopedFdRedirect redirect(request.outputDir + "/retrace.log"); - status = RunRetraceMain(request, usePresentDump); + status = RunRetraceMain(request); } catch (const MobileGLRetraceExit& retraceExit) { status = retraceExit.status; } catch (const std::exception& exception) { @@ -509,54 +415,29 @@ bool RunRetrace(const Request& request, bool usePresentDump, Result& result) { return false; } - std::string snapshotPath = SnapshotPathForCall(request, usePresentDump); - const std::string presentPath = request.outputDir + "/present.ppm"; - const bool hasSnapshot = Exists(snapshotPath); - const bool hasPresentDump = usePresentDump && Exists(presentPath); - if (!hasSnapshot && !hasPresentDump) { + std::string snapshotPath = SnapshotPathForCall(request); + if (!Exists(snapshotPath)) { result.statusCode = STATUS_RETRACE_FAILED; result.message = "retrace completed but did not create expected snapshot: " + snapshotPath; return false; } - if (hasSnapshot && !hasPresentDump) { - RgbaImage snapshot; - std::string imageError; - if (!ReadPngRgba(snapshotPath, snapshot, imageError)) { - result.statusCode = STATUS_IO_ERROR; - result.message = imageError.empty() - ? "failed to decode snapshot PNG: " + snapshotPath - : imageError; - return false; - } - ForceOpaqueAlpha(snapshot); - if (!WritePngRgba(result.actualPath, snapshot, imageError)) { - result.statusCode = STATUS_IO_ERROR; - result.message = imageError.empty() - ? "failed to write snapshot to actual PNG" - : imageError; - return false; - } + + RgbaImage snapshot; + std::string imageError; + if (!ReadPngRgba(snapshotPath, snapshot, imageError)) { + result.statusCode = STATUS_IO_ERROR; + result.message = imageError.empty() + ? "failed to decode snapshot PNG: " + snapshotPath + : imageError; + return false; } - if (usePresentDump) { - if (hasPresentDump) { - RgbaImage present; - std::string imageError; - if (!ReadPpmRgbAsRgba(presentPath, present, imageError) || - !WritePngRgba(request.outputDir + "/present.png", present, imageError)) { - result.statusCode = STATUS_IO_ERROR; - result.message = imageError.empty() - ? "failed to convert DirectVulkan present dump to PNG" - : imageError; - return false; - } - if (!WritePngRgba(result.actualPath, present, imageError)) { - result.statusCode = STATUS_IO_ERROR; - result.message = imageError.empty() - ? "failed to write DirectVulkan present dump to actual PNG" - : imageError; - return false; - } - } + ForceOpaqueAlpha(snapshot); + if (!WritePngRgba(result.actualPath, snapshot, imageError)) { + result.statusCode = STATUS_IO_ERROR; + result.message = imageError.empty() + ? "failed to write snapshot to actual PNG" + : imageError; + return false; } return true; } @@ -927,27 +808,6 @@ Result RunTraceReplay(const Request& request) { return result; } - bool usePresentDump = false; - if (request.backend == "DirectVulkan") { - std::string inspectError; - if (!TargetCallSwapsRenderTarget(request, usePresentDump, inspectError)) { - result.statusCode = STATUS_INVALID_ARGUMENT; - result.message = inspectError; - return result; - } - } - - if (usePresentDump) { - std::string presentDumpPath = request.outputDir + "/present.ppm"; - std::string presentDumpCall = std::to_string(request.targetCall); - setenv("MOBILEGL_PRESENT_DUMP_PATH", presentDumpPath.c_str(), 1); - setenv("MOBILEGL_PRESENT_DUMP_CALL", presentDumpCall.c_str(), 1); - } else { - unsetenv("MOBILEGL_PRESENT_STATS"); - unsetenv("MOBILEGL_PRESENT_DUMP_PATH"); - unsetenv("MOBILEGL_PRESENT_DUMP_CALL"); - } - setenv("MOBILEGL_LOG_FILE_PATH", mobileGlLogPath.c_str(), 1); std::string mobileGlError; @@ -957,7 +817,7 @@ Result RunTraceReplay(const Request& request) { return result; } - if (!RunRetrace(request, usePresentDump, result)) { + if (!RunRetrace(request, result)) { HoldAfterRetrace(request); return result; } diff --git a/tools/trace_replay/apitrace_glws_egl.cpp b/tools/trace_replay/apitrace_glws_egl.cpp index d29b543e..36e1c3f0 100644 --- a/tools/trace_replay/apitrace_glws_egl.cpp +++ b/tools/trace_replay/apitrace_glws_egl.cpp @@ -410,15 +410,8 @@ public: return; } char callNo[32]; - const char *overrideCallNo = getenv("MOBILEGL_TRACE_CURRENT_CALL_OVERRIDE"); - if (overrideCallNo != nullptr && overrideCallNo[0] != '\0') { - snprintf(callNo, sizeof(callNo), "%s", overrideCallNo); - } else { - snprintf(callNo, sizeof(callNo), "%u", retrace::callNo); - } - setenv("MOBILEGL_PRESENT_CURRENT_CALL", callNo, 1); + snprintf(callNo, sizeof(callNo), "%u", retrace::callNo); gEgl.swapBuffers(gDisplay, surface); - unsetenv("MOBILEGL_PRESENT_CURRENT_CALL"); #if defined(__APPLE__) PumpMacOSEvents(); if (window && !windowShown) { diff --git a/tools/trace_replay/skills/mismatch-retrace-debugging.md b/tools/trace_replay/skills/mismatch-retrace-debugging.md index 71826efe..931344ce 100644 --- a/tools/trace_replay/skills/mismatch-retrace-debugging.md +++ b/tools/trace_replay/skills/mismatch-retrace-debugging.md @@ -219,8 +219,8 @@ python3 "$REPO/3rdparty/apitrace/scripts/retracediff.py" \ For MobileGL target state, inspect MobileGL logs and add temporary focused logging in the relevant state sync path. Keep logs scoped to the bad call range -using call-number markers or a `MOBILEGL_TRACE_CURRENT_CALL_OVERRIDE` style -guard if available. +using retrace-side call-number markers rather than backend-specific snapshot +hooks. ## Classify the bad call