Backport context-loss armor for mapped GPU buffers Backports monorepo commit c4874f9ca22ead28f31749e98ddf14acdb32140b. Includes map-failure null checks/unwind in RenderContext and an adapted MapFailureUnwind test for this submodule baseline.
diff --git a/renderer/include/rive/renderer/gpu.hpp b/renderer/include/rive/renderer/gpu.hpp index 6c9ea93..fc22e78 100644 --- a/renderer/include/rive/renderer/gpu.hpp +++ b/renderer/include/rive/renderer/gpu.hpp
@@ -1550,13 +1550,19 @@ using MapResourceBufferFn = void* (RenderContextImpl::*)(size_t mapSizeInBytes); - void mapElements(RenderContextImpl* impl, - MapResourceBufferFn mapFn, - size_t elementCount) + [[nodiscard]] bool mapElements(RenderContextImpl* impl, + MapResourceBufferFn mapFn, + size_t elementCount) { assert(m_mappedMemory == nullptr); void* ptr = (impl->*mapFn)(elementCount * sizeof(T)); + if (ptr == nullptr) + { + return false; + } + reset(reinterpret_cast<T*>(ptr), elementCount); + return true; } using UnmapResourceBufferFn = @@ -1565,10 +1571,12 @@ UnmapResourceBufferFn unmapFn, size_t elementCount) { - assert(m_mappedMemory != nullptr); - assert(m_mappingEnd - m_mappedMemory == elementCount); - (impl->*unmapFn)(elementCount * sizeof(T)); - reset(); + if (m_mappedMemory != nullptr) + { + assert(m_mappingEnd - m_mappedMemory == elementCount); + (impl->*unmapFn)(elementCount * sizeof(T)); + reset(); + } } operator bool() const { return m_mappedMemory; }
diff --git a/renderer/include/rive/renderer/render_context.hpp b/renderer/include/rive/renderer/render_context.hpp index 1557eba..504523d 100644 --- a/renderer/include/rive/renderer/render_context.hpp +++ b/renderer/include/rive/renderer/render_context.hpp
@@ -353,7 +353,7 @@ // size would not change. void setResourceSizes(ResourceAllocationCounts, bool forceRealloc = false); - void mapResourceBuffers(const ResourceAllocationCounts&); + [[nodiscard]] bool mapResourceBuffers(const ResourceAllocationCounts&); void unmapResourceBuffers(const ResourceAllocationCounts&); // Returns the next coverage buffer prefix to use in a logical flush.
diff --git a/renderer/src/render_context.cpp b/renderer/src/render_context.cpp index ef043a5..5d3b691 100644 --- a/renderer/src/render_context.cpp +++ b/renderer/src/render_context.cpp
@@ -770,40 +770,47 @@ m_impl->prepareToFlush(flushResources.currentFrameNumber, flushResources.safeFrameNumber); - - mapResourceBuffers(resourceRequirements); - - for (const auto& flush : m_logicalFlushes) + if (mapResourceBuffers(resourceRequirements)) { - flush->writeResources(); + for (const auto& flush : m_logicalFlushes) + { + flush->writeResources(); + } + + assert(m_flushUniformData.elementsWritten() == m_logicalFlushes.size()); + assert(m_imageDrawUniformData.elementsWritten() == + totalFrameResourceCounts.imageDrawCount); + assert(m_pathData.elementsWritten() == + totalFrameResourceCounts.pathCount + + layoutCounts.pathPaddingCount); + assert(m_paintData.elementsWritten() == + totalFrameResourceCounts.pathCount + + layoutCounts.paintPaddingCount); + assert(m_paintAuxData.elementsWritten() == + totalFrameResourceCounts.pathCount + + layoutCounts.paintAuxPaddingCount); + assert(m_contourData.elementsWritten() == + totalFrameResourceCounts.contourCount + + layoutCounts.contourPaddingCount); + assert(m_gradSpanData.elementsWritten() == + layoutCounts.gradSpanCount + layoutCounts.gradSpanPaddingCount); + assert(m_tessSpanData.elementsWritten() <= + totalFrameResourceCounts.maxTessellatedSegmentCount); + assert(m_triangleVertexData.elementsWritten() <= + totalFrameResourceCounts.maxTriangleVertexCount); + + unmapResourceBuffers(resourceRequirements); + + // Issue logical flushes to the backend. + for (const auto& flush : m_logicalFlushes) + { + m_impl->flush(flush->desc()); + } } - - assert(m_flushUniformData.elementsWritten() == m_logicalFlushes.size()); - assert(m_imageDrawUniformData.elementsWritten() == - totalFrameResourceCounts.imageDrawCount); - assert(m_pathData.elementsWritten() == - totalFrameResourceCounts.pathCount + layoutCounts.pathPaddingCount); - assert(m_paintData.elementsWritten() == - totalFrameResourceCounts.pathCount + layoutCounts.paintPaddingCount); - assert(m_paintAuxData.elementsWritten() == - totalFrameResourceCounts.pathCount + - layoutCounts.paintAuxPaddingCount); - assert(m_contourData.elementsWritten() == - totalFrameResourceCounts.contourCount + - layoutCounts.contourPaddingCount); - assert(m_gradSpanData.elementsWritten() == - layoutCounts.gradSpanCount + layoutCounts.gradSpanPaddingCount); - assert(m_tessSpanData.elementsWritten() <= - totalFrameResourceCounts.maxTessellatedSegmentCount); - assert(m_triangleVertexData.elementsWritten() <= - totalFrameResourceCounts.maxTriangleVertexCount); - - unmapResourceBuffers(resourceRequirements); - - // Issue logical flushes to the backend. - for (const auto& flush : m_logicalFlushes) + else { - m_impl->flush(flush->desc()); + fprintf(stderr, "Buffer mapping failed, cannot render.\n"); + unmapResourceBuffers(resourceRequirements); } m_impl->postFlush(flushResources); @@ -1889,86 +1896,105 @@ m_currentResourceAllocations = allocs; } -void RenderContext::mapResourceBuffers( +bool RenderContext::mapResourceBuffers( const ResourceAllocationCounts& mapCounts) { RIVE_PROF_SCOPE() + +#define HANDLE_MAP_FAILURE(...) \ + do \ + { \ + if (!(__VA_ARGS__)) \ + { \ + return false; \ + } \ + } while (false) + if (mapCounts.flushUniformBufferCount > 0) { - m_flushUniformData.mapElements( + HANDLE_MAP_FAILURE(m_flushUniformData.mapElements( m_impl.get(), &RenderContextImpl::mapFlushUniformBuffer, - mapCounts.flushUniformBufferCount); + mapCounts.flushUniformBufferCount)); } assert(m_flushUniformData.hasRoomFor(mapCounts.flushUniformBufferCount)); if (mapCounts.imageDrawUniformBufferCount > 0) { - m_imageDrawUniformData.mapElements( + HANDLE_MAP_FAILURE(m_imageDrawUniformData.mapElements( m_impl.get(), &RenderContextImpl::mapImageDrawUniformBuffer, - mapCounts.imageDrawUniformBufferCount); + mapCounts.imageDrawUniformBufferCount)); } assert(m_imageDrawUniformData.hasRoomFor( mapCounts.imageDrawUniformBufferCount > 0)); if (mapCounts.pathBufferCount > 0) { - m_pathData.mapElements(m_impl.get(), - &RenderContextImpl::mapPathBuffer, - mapCounts.pathBufferCount); + HANDLE_MAP_FAILURE( + m_pathData.mapElements(m_impl.get(), + &RenderContextImpl::mapPathBuffer, + mapCounts.pathBufferCount)); } assert(m_pathData.hasRoomFor(mapCounts.pathBufferCount)); if (mapCounts.paintBufferCount > 0) { - m_paintData.mapElements(m_impl.get(), - &RenderContextImpl::mapPaintBuffer, - mapCounts.paintBufferCount); + HANDLE_MAP_FAILURE( + m_paintData.mapElements(m_impl.get(), + &RenderContextImpl::mapPaintBuffer, + mapCounts.paintBufferCount)); } assert(m_paintData.hasRoomFor(mapCounts.paintBufferCount)); if (mapCounts.paintAuxBufferCount > 0) { - m_paintAuxData.mapElements(m_impl.get(), - &RenderContextImpl::mapPaintAuxBuffer, - mapCounts.paintAuxBufferCount); + HANDLE_MAP_FAILURE( + m_paintAuxData.mapElements(m_impl.get(), + &RenderContextImpl::mapPaintAuxBuffer, + mapCounts.paintAuxBufferCount)); } assert(m_paintAuxData.hasRoomFor(mapCounts.paintAuxBufferCount)); if (mapCounts.contourBufferCount > 0) { - m_contourData.mapElements(m_impl.get(), - &RenderContextImpl::mapContourBuffer, - mapCounts.contourBufferCount); + HANDLE_MAP_FAILURE( + m_contourData.mapElements(m_impl.get(), + &RenderContextImpl::mapContourBuffer, + mapCounts.contourBufferCount)); } assert(m_contourData.hasRoomFor(mapCounts.contourBufferCount)); if (mapCounts.gradSpanBufferCount > 0) { - m_gradSpanData.mapElements(m_impl.get(), - &RenderContextImpl::mapGradSpanBuffer, - mapCounts.gradSpanBufferCount); + HANDLE_MAP_FAILURE( + m_gradSpanData.mapElements(m_impl.get(), + &RenderContextImpl::mapGradSpanBuffer, + mapCounts.gradSpanBufferCount)); } assert(m_gradSpanData.hasRoomFor(mapCounts.gradSpanBufferCount)); if (mapCounts.tessSpanBufferCount > 0) { - m_tessSpanData.mapElements(m_impl.get(), - &RenderContextImpl::mapTessVertexSpanBuffer, - mapCounts.tessSpanBufferCount); + HANDLE_MAP_FAILURE(m_tessSpanData.mapElements( + m_impl.get(), + &RenderContextImpl::mapTessVertexSpanBuffer, + mapCounts.tessSpanBufferCount)); } assert(m_tessSpanData.hasRoomFor(mapCounts.tessSpanBufferCount)); if (mapCounts.triangleVertexBufferCount > 0) { - m_triangleVertexData.mapElements( + HANDLE_MAP_FAILURE(m_triangleVertexData.mapElements( m_impl.get(), &RenderContextImpl::mapTriangleVertexBuffer, - mapCounts.triangleVertexBufferCount); + mapCounts.triangleVertexBufferCount)); } assert( m_triangleVertexData.hasRoomFor(mapCounts.triangleVertexBufferCount)); + +#undef HANDLE_MAP_FAILURE + return true; } void RenderContext::unmapResourceBuffers(
diff --git a/src/lua/renderer/lua_mesh.cpp b/src/lua/renderer/lua_mesh.cpp index bce924f..7b621eb 100644 --- a/src/lua/renderer/lua_mesh.cpp +++ b/src/lua/renderer/lua_mesh.cpp
@@ -170,11 +170,14 @@ factory->makeRenderBuffer(RenderBufferType::vertex, RenderBufferFlags::mappedOnceAtInitialization, values.size() * sizeof(Vec2D)); - if (buffer) + if (buffer != nullptr) { float* pos = static_cast<float*>(buffer->map()); - memcpy(pos, values.data(), buffer->sizeInBytes()); - buffer->unmap(); + if (pos != nullptr) + { + memcpy(pos, values.data(), buffer->sizeInBytes()); + buffer->unmap(); + } } vertexBuffer = buffer; } @@ -190,11 +193,14 @@ factory->makeRenderBuffer(RenderBufferType::index, RenderBufferFlags::mappedOnceAtInitialization, values.size() * sizeof(uint16_t)); - if (buffer) + if (buffer != nullptr) { void* indexData = buffer->map(); - memcpy(indexData, values.data(), buffer->sizeInBytes()); - buffer->unmap(); + if (indexData != nullptr) + { + memcpy(indexData, values.data(), buffer->sizeInBytes()); + buffer->unmap(); + } } indexBuffer = buffer; }
diff --git a/src/shapes/mesh.cpp b/src/shapes/mesh.cpp index bade491..633704b 100644 --- a/src/shapes/mesh.cpp +++ b/src/shapes/mesh.cpp
@@ -114,16 +114,19 @@ factory->makeRenderBuffer(RenderBufferType::vertex, RenderBufferFlags::mappedOnceAtInitialization, m_Vertices.size() * sizeof(Vec2D)); - if (m_UVRenderBuffer) + if (m_UVRenderBuffer != nullptr) { float* uv = static_cast<float*>(m_UVRenderBuffer->map()); - for (auto vertex : m_Vertices) + if (uv != nullptr) { - Vec2D xformedUV = uvTransform * Vec2D(vertex->u(), vertex->v()); - *uv++ = xformedUV.x; - *uv++ = xformedUV.y; + for (auto vertex : m_Vertices) + { + Vec2D xformedUV = uvTransform * Vec2D(vertex->u(), vertex->v()); + *uv++ = xformedUV.x; + *uv++ = xformedUV.y; + } + m_UVRenderBuffer->unmap(); } - m_UVRenderBuffer->unmap(); } if (m_IndexBuffer != nullptr) @@ -135,10 +138,13 @@ if (m_IndexRenderBuffer) { void* indexData = m_IndexRenderBuffer->map(); - memcpy(indexData, - m_IndexBuffer->data(), - m_IndexRenderBuffer->sizeInBytes()); - m_IndexRenderBuffer->unmap(); + if (indexData != nullptr) + { + memcpy(indexData, + m_IndexBuffer->data(), + m_IndexRenderBuffer->sizeInBytes()); + m_IndexRenderBuffer->unmap(); + } } } } @@ -176,11 +182,15 @@ { Vec2D* mappedVertices = reinterpret_cast<Vec2D*>(m_VertexRenderBuffer->map()); - for (auto vertex : m_Vertices) + if (mappedVertices != nullptr) { - *mappedVertices++ = vertex->renderTranslation(); + for (auto vertex : m_Vertices) + { + *mappedVertices++ = vertex->renderTranslation(); + } + m_VertexRenderBuffer->unmap(); } - m_VertexRenderBuffer->unmap(); + m_VertexRenderBufferDirty = false; }
diff --git a/src/shapes/slice_mesh.cpp b/src/shapes/slice_mesh.cpp index 22e96b2..39de547 100644 --- a/src/shapes/slice_mesh.cpp +++ b/src/shapes/slice_mesh.cpp
@@ -73,15 +73,18 @@ vertexSizeInBytes); } - if (m_VertexRenderBuffer) + if (m_VertexRenderBuffer != nullptr) { Vec2D* mappedVertices = reinterpret_cast<Vec2D*>(m_VertexRenderBuffer->map()); - for (auto v : m_vertices) + if (mappedVertices != nullptr) { - *mappedVertices++ = v; + for (auto v : m_vertices) + { + *mappedVertices++ = v; + } + m_VertexRenderBuffer->unmap(); } - m_VertexRenderBuffer->unmap(); } // 2. uv render buffer @@ -106,12 +109,15 @@ renderImage != nullptr ? renderImage->uvTransform() : Mat2D(); Vec2D* mappedUVs = reinterpret_cast<Vec2D*>(m_UVRenderBuffer->map()); - for (auto uv : m_uvs) + if (mappedUVs != nullptr) { - Vec2D xformedUV = uvTransform * uv; - *mappedUVs++ = xformedUV; + for (auto uv : m_uvs) + { + Vec2D xformedUV = uvTransform * uv; + *mappedUVs++ = xformedUV; + } + m_UVRenderBuffer->unmap(); } - m_UVRenderBuffer->unmap(); } // 3. index render buffer @@ -132,8 +138,11 @@ if (m_IndexRenderBuffer) { void* mappedIndex = m_IndexRenderBuffer->map(); - memcpy(mappedIndex, m_indices.data(), indexSizeInBytes); - m_IndexRenderBuffer->unmap(); + if (mappedIndex != nullptr) + { + memcpy(mappedIndex, m_indices.data(), indexSizeInBytes); + m_IndexRenderBuffer->unmap(); + } } }
diff --git a/tests/unit_tests/renderer/pls_render_context_test.cpp b/tests/unit_tests/renderer/pls_render_context_test.cpp index 89568cf..56474f9 100644 --- a/tests/unit_tests/renderer/pls_render_context_test.cpp +++ b/tests/unit_tests/renderer/pls_render_context_test.cpp
@@ -35,6 +35,77 @@ } }; +class RenderContextNULLTestForMapFail : public RenderContextNULL +{ + using Super = RenderContextNULL; + +public: + static constexpr auto MAP_COUNT = 9u; + + explicit RenderContextNULLTestForMapFail(uint32_t failingMapIndex) : + Super(), m_failingMapIndex(failingMapIndex) + {} + + size_t totalMapCount() const { return m_totalMapCount; } + +#define MAKE_MAP_UNMAP(index, Name) \ + void* map##Name(size_t mapSizeInBytes) override \ + { \ + static_assert(index < MAP_COUNT); \ + assert(m_mapCounts[index] == 0u); \ + if (index == m_failingMapIndex) \ + { \ + return nullptr; \ + } \ + \ + m_mapCounts[index] = mapSizeInBytes; \ + m_totalMapCount += mapSizeInBytes; \ + return Super::map##Name(mapSizeInBytes); \ + } \ + \ + void unmap##Name(size_t mapSizeInBytes) override \ + { \ + if (m_mapCounts[index] != 0u) \ + { \ + assert(m_mapCounts[index] == mapSizeInBytes); \ + Super::unmap##Name(mapSizeInBytes); \ + m_mapCounts[index] = 0u; \ + m_totalMapCount -= mapSizeInBytes; \ + } \ + } + + MAKE_MAP_UNMAP(0, FlushUniformBuffer) + MAKE_MAP_UNMAP(1, ImageDrawUniformBuffer) + MAKE_MAP_UNMAP(2, PathBuffer) + MAKE_MAP_UNMAP(3, PaintBuffer) + MAKE_MAP_UNMAP(4, PaintAuxBuffer) + MAKE_MAP_UNMAP(5, ContourBuffer) + MAKE_MAP_UNMAP(6, GradSpanBuffer) + MAKE_MAP_UNMAP(7, TessVertexSpanBuffer) + MAKE_MAP_UNMAP(8, TriangleVertexBuffer) + +#undef MAKE_MAP_UNMAP + +private: + size_t m_mapCounts[MAP_COUNT]{}; + uint32_t m_failingMapIndex = 0; + size_t m_totalMapCount = 0; +}; + +class RenderContextTestForMapFail : public rive::gpu::RenderContext +{ +public: + explicit RenderContextTestForMapFail(uint32_t failingMapIndex) : + RenderContext(std::make_unique<RenderContextNULLTestForMapFail>( + failingMapIndex)) + {} + + RenderContextNULLTestForMapFail* testingImpl() + { + return static_impl_cast<RenderContextNULLTestForMapFail>(); + } +}; + namespace rive::gpu { TEST_CASE("ResourceAllocationCounts", "RenderContext") @@ -224,4 +295,36 @@ CHECK(ctx.currentResourceAllocations().gradTextureHeight == 0); CHECK(ctx.currentResourceAllocations().tessTextureHeight == 0); } + +TEST_CASE("MapFailureUnwind", "RenderContext") +{ + for (auto failIndex = 0u; failIndex < RenderContextNULLTestForMapFail::MAP_COUNT; + failIndex++) + { + RenderContextTestForMapFail ctx{failIndex}; + rive::RiveRenderer renderer(&ctx); + + auto twoContourPath = ctx.makeEmptyRenderPath(); + twoContourPath->addRect(0, 0, 100, 100); + twoContourPath->addRect(20, 20, 80, 80); + + auto paint = ctx.makeRenderPaint(); + + auto makeSimpleFrameDescriptor = []() { + RenderContext::FrameDescriptor desc = { + .renderTargetWidth = 200, + .renderTargetHeight = 200, + }; + return desc; + }; + + auto renderTarget = ctx.testingImpl()->makeRenderTarget(200, 200); + + ctx.beginFrame(makeSimpleFrameDescriptor()); + renderer.drawPath(twoContourPath.get(), paint.get()); + ctx.flush({.renderTarget = renderTarget.get()}); + + CHECK(ctx.testingImpl()->totalMapCount() == 0); + } +} } // namespace rive::gpu