fix(Vulkan): Various Android Vulkan fixes (#10927) a7d233d7c1

Device fixes:
- Break the MSAA resolve off into a separate render subpass to resolve corruption on some Adreno devices
- Work around an issue where some early Mali drivers report Vulkan 1.1 support but are missing the allocation functions that vma is looking for
- Some of those Mali devices spew incorrect validation errors about the inability to create, say, an RGB8 texture because the maximum mipmap count is 0 (and other similar "oops we queried and got 0" errors) even though the images actually create fine. Added them to an ignore list.
- The MSAA color and depth/stencil textures are now being created with the TRANSIENT bit (this doesn't fix any known device issues but it does seem more correct)

Other improvements:
- vk_check now displays a string representation of the vulkan error instead of just its numeric value
- the abort handler on Android now also prints the stack trace (to make debugging easier)
- fiddle_context_vulkan now requests the correct width and height for image capture
- The queueImageCopy function in swapchain was not properly setting the pixel read bounds to the whole swapchain if passed an empty AABB

Co-authored-by: Josh Jersild <joshua@rive.app>
This commit is contained in:
JoshJRive
2025-10-30 02:17:02 +00:00
parent bd60875016
commit 5749982961
12 changed files with 239 additions and 38 deletions

View File

@@ -1 +1 @@
a406176fa69a0cb005fb09453cbd535ce3409496
a7d233d7c185324bd4d30ba1c95f571b9466133c

View File

@@ -22,12 +22,15 @@ class VulkanContext;
namespace rive::gpu::vkutil
{
const char* string_from_vk_result(VkResult);
inline static void vk_check(VkResult res, const char* file, int line)
{
if (res != VK_SUCCESS)
{
fprintf(stderr,
"Vulkan error %i at line: %i in file: %s\n",
"Vulkan error %s (%i) at line: %i in file: %s\n",
string_from_vk_result(res),
res,
line,
file);

View File

@@ -264,7 +264,10 @@ public:
auto lastAccess = m_renderTarget->targetLastAccess();
if (pixelData != nullptr)
{
m_swapchain->queueImageCopy(&lastAccess);
m_swapchain->queueImageCopy(
&lastAccess,
rive::IAABB::MakeWH(m_renderTarget->width(),
m_renderTarget->height()));
}
m_swapchain->endFrame(lastAccess);

View File

@@ -3,6 +3,8 @@
*/
#include <array>
#include <assert.h>
#include <atomic>
#include <stdlib.h>
#include <string.h>
#include "logging.hpp"
@@ -10,7 +12,40 @@
namespace rive_vkb
{
static bool shouldErrorMessageAbort(const char* message)
// Some messages should be fully ignored (because they are erroneous and would
// otherwise fill the logs)
static bool should_error_message_be_fully_ignored(const char* message)
{
#ifdef RIVE_ANDROID
// Some Mali drivers (specifically the Mali-G71) will report spurious
// resource limit errors/warnings where it incorrectly thinks the maximum
// size is 0.
// For example: "pCreateInfo->mipLevels (1) exceed image format
// maxMipLevels (0) for format VK_FORMAT_R8G8B8A8_UNORM."
static constexpr const char* s_ignoreList[] = {
"exceeds allowable maximum image extent width 0 ",
"exceeds allowable maximum image extent height 0 ",
"exceeds allowable maximum image extent depth 0 ",
"exceed image format maxMipLevels (0) ",
" (format maxArrayLayers: 0)",
"maximum resource size = 0 for format ",
"(format sampleCounts: VkSampleCountFlags(0))",
};
for (const char* ignore : s_ignoreList)
{
if (strstr(message, ignore) != nullptr)
{
return true;
}
}
#endif
return false;
}
static bool should_error_message_abort(const char* message)
{
static std::array<const char*, 3> s_ignoredValidationMsgList = {
// Swiftshader generates this error during
@@ -43,11 +78,15 @@ VKAPI_ATTR VkBool32 VKAPI_CALL defaultDebugUtilCallback(
void* pUserData)
{
bool shouldAbort = true;
if (should_error_message_be_fully_ignored(pCallbackData->pMessage))
{
return VK_TRUE;
}
switch (messageType)
{
case VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT:
shouldAbort = shouldErrorMessageAbort(pCallbackData->pMessage);
shouldAbort = should_error_message_abort(pCallbackData->pMessage);
LOG_ERROR_LINE("Rive Vulkan error%s: %i: %s: %s\n",
(shouldAbort) ? "" : " (error ignored)",
pCallbackData->messageIdNumber,
@@ -86,9 +125,14 @@ defaultDebugReportCallback(VkDebugReportFlagsEXT flags,
const char* message,
void* pUserData)
{
if (should_error_message_be_fully_ignored(message))
{
return false;
}
if ((flags & VK_DEBUG_REPORT_ERROR_BIT_EXT) != 0)
{
bool shouldAbort = shouldErrorMessageAbort(message);
bool shouldAbort = should_error_message_abort(message);
LOG_ERROR_LINE("Rive Vulkan error%s: %s (%i): %s",
(shouldAbort) ? "" : " (error ignored)",

View File

@@ -285,7 +285,7 @@ VulkanInstance::VulkanInstance(const Options& opts)
}
}
}
}; // namespace rive_vkb
}
VulkanInstance::~VulkanInstance()
{

View File

@@ -202,6 +202,10 @@ void VulkanSwapchain::queueImageCopy(
rive::gpu::vkutil::ImageAccess* inOutLastAccess,
rive::IAABB optPixelReadBounds)
{
if (optPixelReadBounds.empty())
{
optPixelReadBounds = rive::IAABB::MakeWH(m_width, m_height);
}
queueImageCopy(current().image,
m_imageFormat,
inOutLastAccess,

View File

@@ -2369,6 +2369,16 @@ void RenderContextVulkanImpl::flush(const FlushDescriptor& desc)
}
}
if (desc.interlockMode == InterlockMode::msaa)
{
// MSAA needs a follow-up subpass to resolve the MSAA buffer to the
// single-sampled render target. No actually rendering is necessary, it
// is taken care of entirely via the render pass mechanisms. This is
// a workaround for what appears to be a driver bug in some early Adreno
// drivers.
m_vk->CmdNextSubpass(commandBuffer, VK_SUBPASS_CONTENTS_INLINE);
}
m_vk->CmdEndRenderPass(commandBuffer);
if (colorAttachmentIsOffscreen &&

View File

@@ -368,12 +368,19 @@ RenderPassVulkan::RenderPassVulkan(
interlockMode == gpu::InterlockMode::rasterOrdering &&
m_vk->features.rasterizationOrderColorAttachmentAccess;
constexpr uint32_t MAX_SUBPASSES = 2;
constexpr uint32_t MAX_SUBPASSES = 3;
StackVector<VkSubpassDescription, MAX_SUBPASSES> subpassDescs;
constexpr uint32_t MAX_SUBPASS_DEPS = MAX_SUBPASSES + 1;
StackVector<VkSubpassDependency, MAX_SUBPASS_DEPS> subpassDeps;
// MSAA resolve needs its own set of input references vs.
// inputAttachmentRefs, specifically because the color buffer always needs
// to be used for the resolve (whereas normally it's occasioanlly set to
// VK_ATTACHMENT_UNUSED because we only use it when doing advanced blend)
StackVector<VkAttachmentReference, PLS_PLANE_COUNT>
msaaResolveInputAttachmentRefs;
// MSAA color-load subpass.
if (interlockMode == gpu::InterlockMode::msaa &&
loadAction == gpu::LoadAction::preserveRenderTarget)
@@ -419,9 +426,32 @@ RenderPassVulkan::RenderPassVulkan(
.pInputAttachments = inputAttachmentRefs.data(),
.colorAttachmentCount = colorAttachmentRefs.size(),
.pColorAttachments = colorAttachmentRefs.data(),
.pResolveAttachments = msaaResolveAttachmentRef.dataOrNull(),
.pDepthStencilAttachment = depthStencilAttachmentRef.dataOrNull(),
});
if (msaaResolveAttachmentRef.size() > 0)
{
// Some Android drivers (some Android 12 and earlier Adreno drivers)
// have issues with having both a self-dependency and resolve
// attachments. The resolve can instead be done as a second pass (in
// which no actual rendering occurs), which eliminates some corruption
// during blending on the affected devices.
msaaResolveInputAttachmentRefs.push_back_n(colorAttachmentRefs.size(),
colorAttachmentRefs.data());
msaaResolveInputAttachmentRefs[0].layout = VK_IMAGE_LAYOUT_GENERAL;
subpassDescs.push_back({
.flags = 0u,
.pipelineBindPoint = VK_PIPELINE_BIND_POINT_GRAPHICS,
.inputAttachmentCount = msaaResolveInputAttachmentRefs.size(),
.pInputAttachments = msaaResolveInputAttachmentRefs.data(),
.colorAttachmentCount = colorAttachmentRefs.size(),
.pColorAttachments = colorAttachmentRefs.data(),
.pResolveAttachments = msaaResolveAttachmentRef.data(),
});
}
if ((interlockMode == gpu::InterlockMode::rasterOrdering &&
!rasterOrderedAttachmentAccess) ||
interlockMode == gpu::InterlockMode::atomics ||

View File

@@ -111,6 +111,7 @@ vkutil::Texture2D* RenderTargetVulkan::msaaColorTexture()
.extent = {width(), height(), 1},
.samples = VK_SAMPLE_COUNT_4_BIT,
.usage = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT |
VK_IMAGE_USAGE_TRANSIENT_ATTACHMENT_BIT |
VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT,
});
}
@@ -126,7 +127,8 @@ vkutil::Texture2D* RenderTargetVulkan::msaaDepthStencilTexture()
m_vk->supportsD24S8()),
.extent = {width(), height(), 1},
.samples = VK_SAMPLE_COUNT_4_BIT,
.usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT,
.usage = VK_IMAGE_USAGE_DEPTH_STENCIL_ATTACHMENT_BIT |
VK_IMAGE_USAGE_TRANSIENT_ATTACHMENT_BIT,
});
}
return m_msaaDepthStencilTexture.get();

View File

@@ -10,6 +10,41 @@
namespace rive::gpu::vkutil
{
#define STR_CASE(result) \
case VK_##result: \
return #result
const char* string_from_vk_result(VkResult result)
{
switch (result)
{
STR_CASE(SUCCESS);
STR_CASE(NOT_READY);
STR_CASE(TIMEOUT);
STR_CASE(EVENT_SET);
STR_CASE(EVENT_RESET);
STR_CASE(INCOMPLETE);
STR_CASE(ERROR_OUT_OF_HOST_MEMORY);
STR_CASE(ERROR_OUT_OF_DEVICE_MEMORY);
STR_CASE(ERROR_INITIALIZATION_FAILED);
STR_CASE(ERROR_DEVICE_LOST);
STR_CASE(ERROR_MEMORY_MAP_FAILED);
STR_CASE(ERROR_LAYER_NOT_PRESENT);
STR_CASE(ERROR_EXTENSION_NOT_PRESENT);
STR_CASE(ERROR_FEATURE_NOT_PRESENT);
STR_CASE(ERROR_INCOMPATIBLE_DRIVER);
STR_CASE(ERROR_TOO_MANY_OBJECTS);
STR_CASE(ERROR_FORMAT_NOT_SUPPORTED);
STR_CASE(ERROR_SURFACE_LOST_KHR);
STR_CASE(SUBOPTIMAL_KHR);
STR_CASE(ERROR_OUT_OF_DATE_KHR);
STR_CASE(ERROR_INCOMPATIBLE_DISPLAY_KHR);
STR_CASE(ERROR_NATIVE_WINDOW_IN_USE_KHR);
STR_CASE(ERROR_VALIDATION_FAILED_EXT);
default:
return "<unknown>";
}
}
Resource::Resource(rcp<VulkanContext> vk) : GPUResource(std::move(vk)) {}
VulkanContext* Resource::vk() const

View File

@@ -19,6 +19,27 @@ static VmaAllocator make_vma_allocator(
.vkGetDeviceProcAddr = vk->GetDeviceProcAddr,
.vkGetPhysicalDeviceProperties = vk->GetPhysicalDeviceProperties,
};
uint32_t vmaAPIVersion = vk->features.apiVersion;
if (vmaAPIVersion > VK_API_VERSION_1_0)
{
const auto vkGetBufferMemoryRequirements2 =
vk->GetDeviceProcAddr(vk->device, "vkGetBufferMemoryRequirements2");
const auto vkGetImageMemoryRequirements2 =
vk->GetDeviceProcAddr(vk->device, "vkGetImageMemoryRequirements2");
if (vkGetBufferMemoryRequirements2 == nullptr ||
vkGetImageMemoryRequirements2 == nullptr)
{
// Some Android drivers don't have these functions even though they
// say they're Vulkan 1.1 (which included these functions standard),
// so tell VMA it's 1.0 in those cases so it doesn't fail trying to
// use them.
vmaAPIVersion = VK_API_VERSION_1_0;
}
}
VmaAllocatorCreateInfo vmaCreateInfo = {
// We are single-threaded.
.flags = VMA_ALLOCATOR_CREATE_EXTERNALLY_SYNCHRONIZED_BIT,
@@ -26,7 +47,7 @@ static VmaAllocator make_vma_allocator(
.device = vk->device,
.pVulkanFunctions = &vmaVulkanFunctions,
.instance = vk->instance,
.vulkanApiVersion = vk->features.apiVersion,
.vulkanApiVersion = vmaAPIVersion,
};
VK_CHECK(vmaCreateAllocator(&vmaCreateInfo, &vmaAllocator));
return vmaAllocator;

View File

@@ -20,6 +20,11 @@ void replace_signal_handlers(SignalFunc signalFunc,
#include <Windows.h>
#include <dbghelp.h>
#else
#include <inttypes.h>
#ifdef RIVE_ANDROID
#include <unwind.h>
#include <android/log.h>
#endif
#include <execinfo.h>
#include <string.h>
#include <errno.h>
@@ -196,15 +201,80 @@ void replace_signal_handlers(SignalFunc inSignalFunc,
atexit(atExitFunc);
}
#else
static void get_symbol_line(void* instructionPointer,
uint32_t i,
char* buffer,
std::size_t bufferSize)
{
Dl_info dlInfo;
if (dladdr(instructionPointer, &dlInfo) && dlInfo.dli_sname != nullptr)
{
char* demangled = nullptr;
int status;
demangled = abi::__cxa_demangle(dlInfo.dli_sname, nullptr, 0, &status);
snprintf(buffer,
bufferSize,
"%u %s\n",
i,
(status == 0 && demangled != nullptr) ? demangled
: dlInfo.dli_sname);
free(demangled);
}
else
{
snprintf(buffer,
bufferSize,
"%u <unknown symbol @%p>)\n",
i,
instructionPointer);
}
// snprintf does not null terminate if we hit the limit so add a null
// terminator here to be safe.
buffer[bufferSize - 1] = '\0';
}
template <std::size_t N>
static void get_symbol_line(void* instructionPointer,
uint32_t i,
char (&buffer)[N])
{
get_symbol_line(instructionPointer, i, buffer, N);
}
#ifdef RIVE_ANDROID
// android is a little more complicated in that it requires specific compiler
// flags that i don't want to force us into, so we just do it the old way for
// now
#define LOG(...) \
__android_log_print(ANDROID_LOG_ERROR, "rive_android_tests", __VA_ARGS__)
static void handle_signal(int sigNum, siginfo_t* signalInfo, void* userContext)
{
printf("Received signal %i (\"%s\")\n", sigNum, strsignal(sigNum));
LOG("Received signal %i (\"%s\")\n", sigNum, strsignal(sigNum));
LOG("Stack trace:\n");
struct State
{
uint32_t i = 0;
std::stringstream f;
};
State st{};
_Unwind_Backtrace(
[](struct _Unwind_Context* context, void* stateVoid) {
auto state = static_cast<State*>(stateVoid);
void* ip = reinterpret_cast<void*>(_Unwind_GetIP(context));
if (ip != nullptr)
{
char buffer[1024];
get_symbol_line(ip, state->i, buffer);
LOG("%s", buffer);
state->f << buffer;
}
state->i++;
return _URC_NO_REASON;
},
&st);
signal(sigNum, SIG_DFL);
signalFunc(strsignal(sigNum));
signalFunc(st.f.str().c_str());
raise(sigNum);
}
@@ -235,28 +305,7 @@ static void handle_signal(int sigNum, siginfo_t* signalInfo, void* userContext)
symbols = backtrace_symbols(stacktrace, numFrames);
for (int i = 1; i < numFrames; i++)
{
Dl_info dlInfo;
if (dladdr(stacktrace[i], &dlInfo))
{
char* demangled = NULL;
int status;
demangled = abi::__cxa_demangle(dlInfo.dli_sname, NULL, 0, &status);
snprintf(stringBuff,
sizeof(stringBuff),
"%d %s\n",
i,
status == 0 ? demangled : dlInfo.dli_sname);
free(demangled);
}
else
{
snprintf(stringBuff,
sizeof(stringBuff),
"%d %p\n",
i,
stacktrace[i]);
}
get_symbol_line(stacktrace[i], i, stringBuff, std::size(stringBuff));
f << stringBuff << symbols[i] << "\n";
}