From bc378ad135a528b9cb97fd12049b38413c45cb3a Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Fri, 23 Jul 2021 08:08:47 +0530 Subject: [PATCH] Fix `GraphicBufferProducer` Format Bug The format provided in `GraphicBuffer` can be misleading and is supplied as `None` by the Deko3D Swapchain, it instead supplies the real format in the `NvGraphicHandle` which we now utilize instead of the one in `GraphicBuffer`. --- .../hosbinder/GraphicBufferProducer.cpp | 30 ++++++++++--------- .../services/hosbinder/android_types.h | 5 ++-- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp index cbe103e3..8f3b2199 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp +++ b/app/src/main/cpp/skyline/services/hosbinder/GraphicBufferProducer.cpp @@ -138,8 +138,9 @@ namespace skyline::service::hosbinder { // All buffers must be preallocated on the client application and attached to an Android buffer using SetPreallocatedBuffer return AndroidStatus::NoMemory; } - auto &surface{buffer->graphicBuffer->graphicHandle.surfaces.front()}; - if (buffer->graphicBuffer->format != format || surface.width != width || surface.height != height || (buffer->graphicBuffer->usage & usage) != usage) { + auto &handle{buffer->graphicBuffer->graphicHandle}; + auto &surface{handle.surfaces.front()}; + if (handle.format != format || surface.width != width || surface.height != height || (buffer->graphicBuffer->usage & usage) != usage) { state.logger->Warn("Buffer which has been dequeued isn't compatible with the supplied parameters: Dimensions: {}x{}={}x{}, Format: {}={}, Usage: 0x{:X}=0x{:X}", width, height, surface.width, surface.height, ToString(format), ToString(buffer->graphicBuffer->format), usage, buffer->graphicBuffer->usage); // Nintendo doesn't deallocate the slot which was picked in here and reallocate it as a compatible buffer // This is related to the comment above, Nintendo only allocates buffers on the client side @@ -264,7 +265,7 @@ namespace skyline::service::hosbinder { preallocatedBufferCount = std::count_if(queue.begin(), queue.end(), [](const BufferSlot &slot) { return slot.graphicBuffer && slot.isPreallocated; }); activeSlotCount = std::count_if(queue.begin(), queue.end(), [](const BufferSlot &slot) { return slot.graphicBuffer != nullptr; }); - state.logger->Debug("#{} - Dimensions: {}x{} [Stride: {}], Format: {}, Layout: {}, {}: {}, Usage: 0x{:X}, NvMap {}: {}, Buffer Start/End: 0x{:X} -> 0x{:X}", slot, surface.width, surface.height, handle.stride, ToString(graphicBuffer.format), ToString(surface.layout), surface.layout == NvSurfaceLayout::Blocklinear ? "Block Height" : "Pitch", surface.layout == NvSurfaceLayout::Blocklinear ? 1U << surface.blockHeightLog2 : surface.pitch, graphicBuffer.usage, surface.nvmapHandle ? "Handle" : "ID", surface.nvmapHandle ? surface.nvmapHandle : handle.nvmapId, surface.offset, surface.offset + surface.size); + state.logger->Debug("#{} - Dimensions: {}x{} [Stride: {}], Format: {}, Layout: {}, {}: {}, Usage: 0x{:X}, NvMap {}: {}, Buffer Start/End: 0x{:X} -> 0x{:X}", slot, surface.width, surface.height, handle.stride, ToString(handle.format), ToString(surface.layout), surface.layout == NvSurfaceLayout::Blocklinear ? "Block Height" : "Pitch", surface.layout == NvSurfaceLayout::Blocklinear ? 1U << surface.blockHeightLog2 : surface.pitch, graphicBuffer.usage, surface.nvmapHandle ? "Handle" : "ID", surface.nvmapHandle ? surface.nvmapHandle : handle.nvmapId, surface.offset, surface.offset + surface.size); return AndroidStatus::Ok; } @@ -305,8 +306,17 @@ namespace skyline::service::hosbinder { if (!buffer.texture) [[unlikely]] { // We lazily create a texture if one isn't present at queue time, this allows us to look up the texture in the texture cache // If we deterministically know that the texture is written by the CPU then we can allocate a CPU-shared host texture for fast uploads + + auto &handle{graphicBuffer.graphicHandle}; + if (handle.magic != NvGraphicHandle::Magic) + throw exception("Unexpected NvGraphicHandle magic: {}", handle.surfaceCount); + else if (handle.surfaceCount < 1) + throw exception("At least one surface is required in a buffer: {}", handle.surfaceCount); + else if (handle.surfaceCount > 1) + throw exception("Multi-planar surfaces are not supported: {}", handle.surfaceCount); + gpu::texture::Format format; - switch (graphicBuffer.format) { + switch (handle.format) { case AndroidPixelFormat::RGBA8888: case AndroidPixelFormat::RGBX8888: format = gpu::format::RGBA8888Unorm; @@ -317,17 +327,9 @@ namespace skyline::service::hosbinder { break; default: - throw exception("Unknown format in buffer: '{}' ({})", ToString(graphicBuffer.format), static_cast(graphicBuffer.format)); + throw exception("Unknown format in buffer: '{}' ({})", ToString(handle.format), static_cast(handle.format)); } - auto &handle{graphicBuffer.graphicHandle}; - if (handle.magic != NvGraphicHandle::Magic) - throw exception("Unexpected NvGraphicHandle magic: {}", handle.surfaceCount); - else if (handle.surfaceCount < 1) - throw exception("At least one surface is required in a buffer: {}", handle.surfaceCount); - else if (handle.surfaceCount > 1) - throw exception("Multi-planar surfaces are not supported: {}", handle.surfaceCount); - auto &surface{graphicBuffer.graphicHandle.surfaces.at(0)}; if (surface.scanFormat != NvDisplayScanFormat::Progressive) throw exception("Non-Progressive surfaces are not supported: {}", ToString(surface.scanFormat)); @@ -595,7 +597,7 @@ namespace skyline::service::hosbinder { else if (surface.layout == NvSurfaceLayout::Tiled) throw exception("Legacy 16Bx16 tiled surfaces are not supported"); - state.logger->Debug("#{} - Dimensions: {}x{} [Stride: {}], Format: {}, Layout: {}, {}: {}, Usage: 0x{:X}, NvMap {}: {}, Buffer Start/End: 0x{:X} -> 0x{:X}", slot, surface.width, surface.height, handle.stride, ToString(graphicBuffer->format), ToString(surface.layout), surface.layout == NvSurfaceLayout::Blocklinear ? "Block Height" : "Pitch", surface.layout == NvSurfaceLayout::Blocklinear ? 1U << surface.blockHeightLog2 : surface.pitch, graphicBuffer->usage, surface.nvmapHandle ? "Handle" : "ID", surface.nvmapHandle ? surface.nvmapHandle : handle.nvmapId, surface.offset, surface.offset + surface.size); + state.logger->Debug("#{} - Dimensions: {}x{} [Stride: {}], Format: {}, Layout: {}, {}: {}, Usage: 0x{:X}, NvMap {}: {}, Buffer Start/End: 0x{:X} -> 0x{:X}", slot, surface.width, surface.height, handle.stride, ToString(handle.format), ToString(surface.layout), surface.layout == NvSurfaceLayout::Blocklinear ? "Block Height" : "Pitch", surface.layout == NvSurfaceLayout::Blocklinear ? 1U << surface.blockHeightLog2 : surface.pitch, graphicBuffer->usage, surface.nvmapHandle ? "Handle" : "ID", surface.nvmapHandle ? surface.nvmapHandle : handle.nvmapId, surface.offset, surface.offset + surface.size); } else { state.logger->Debug("#{} - No GraphicBuffer", slot); } diff --git a/app/src/main/cpp/skyline/services/hosbinder/android_types.h b/app/src/main/cpp/skyline/services/hosbinder/android_types.h index df3668a2..9df8826f 100644 --- a/app/src/main/cpp/skyline/services/hosbinder/android_types.h +++ b/app/src/main/cpp/skyline/services/hosbinder/android_types.h @@ -188,7 +188,7 @@ namespace skyline::service::hosbinder { static_assert(sizeof(NvSurface) == 0x58); /** - * @brief The integers of the native_handle used by Nvidia to marshall the surfaces in this buffer + * @brief The format of the native_handle integers used by Nvidia to marshall the surface metadata of the GraphicBuffer */ struct NvGraphicHandle { constexpr static u32 Magic{0xDAFFCAFF}; @@ -199,7 +199,7 @@ namespace skyline::service::hosbinder { u32 ownerPid; //!< Same as the upper 32-bits of the ID in the GraphicBuffer (0x2F) u32 type; u32 usage; //!< The Gralloc usage flags, same as GraphicBuffer - u32 format; //!< The internal format of the buffer + AndroidPixelFormat format; //!< The internal format of the buffer u32 externalFormat; //!< The external format that's exposed by Gralloc u32 stride; u32 size; //!< The size of the buffer in bytes @@ -213,6 +213,7 @@ namespace skyline::service::hosbinder { /** * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/include/ui/GraphicBuffer.h * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/libs/ui/GraphicBuffer.cpp;l=266-301 + * @note None of the values from here should really be used aside from Gralloc usage, the ones in NvGraphicHandle take precedence */ struct GraphicBuffer { constexpr static u32 Magic{'GBFR'}; //!< The magic is in little-endian, we do not need to use 'util::MakeMagic'