From ca84b6db488237f2b3ccaa6528eaa35388c79a56 Mon Sep 17 00:00:00 2001 From: Alex Szpakowski Date: Wed, 5 Feb 2020 07:49:38 -0400 Subject: [PATCH 1/3] More robust error handling for AMD Pinned Memory buffers (issue #1540) Also print out any errors that are produced from that (for now). --- src/modules/graphics/opengl/StreamBuffer.cpp | 31 ++++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/modules/graphics/opengl/StreamBuffer.cpp b/src/modules/graphics/opengl/StreamBuffer.cpp index 605bcd31b..7e2590417 100644 --- a/src/modules/graphics/opengl/StreamBuffer.cpp +++ b/src/modules/graphics/opengl/StreamBuffer.cpp @@ -406,7 +406,12 @@ public: if (!alignedMalloc((void **) &data, alignedSize, alignment)) throw love::Exception("Out of memory."); - loadVolatile(); + if (!loadVolatile()) + { + ptrdiff_t pointer = (ptrdiff_t) data; + alignedFree(data); + throw love::Exception("AMD Pinned Memory StreamBuffer implementation failed to create buffer (address: %p, alignment: %ld, aiigned size: %ld)", pointer, alignment, alignedSize); + } } ~StreamBufferPinnedMemory() @@ -441,9 +446,19 @@ public: glGenBuffers(1, &vbo); + while (glGetError() != GL_NO_ERROR) + /* Clear errors. */; + glBindBuffer(GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD, vbo); glBufferData(GL_EXTERNAL_VIRTUAL_MEMORY_BUFFER_AMD, alignedSize, data, GL_STREAM_DRAW); + if (glGetError() != GL_NO_ERROR) + { + gl.deleteBuffer(vbo); + vbo = 0; + return false; + } + frameGPUReadOffset = 0; frameIndex = 0; @@ -485,8 +500,18 @@ love::graphics::StreamBuffer *CreateStreamBuffer(BufferType mode, size_t size) // AMD's pinned memory seems to be faster than persistent mapping, // on AMD GPUs. if (GLAD_AMD_pinned_memory) - return new StreamBufferPinnedMemory(mode, size); - else if (GLAD_VERSION_4_4 || GLAD_ARB_buffer_storage) + { + try + { + return new StreamBufferPinnedMemory(mode, size); + } + catch (love::Exception &e) + { + printf("Failed creating Pinned Memory StreamBuffer: %s\n", e.what()); + } + } + + if (GLAD_VERSION_4_4 || GLAD_ARB_buffer_storage) return new StreamBufferPersistentMapSync(mode, size); // Most modern drivers have a separate internal thread which queues From 813b1e0e69852e7744e3a9e6e4855be22f085c31 Mon Sep 17 00:00:00 2001 From: Alex Szpakowski Date: Wed, 5 Feb 2020 18:03:34 -0400 Subject: [PATCH 2/3] Remove debug print --- src/modules/graphics/opengl/StreamBuffer.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/graphics/opengl/StreamBuffer.cpp b/src/modules/graphics/opengl/StreamBuffer.cpp index 7e2590417..8ea3a39a8 100644 --- a/src/modules/graphics/opengl/StreamBuffer.cpp +++ b/src/modules/graphics/opengl/StreamBuffer.cpp @@ -507,7 +507,11 @@ love::graphics::StreamBuffer *CreateStreamBuffer(BufferType mode, size_t size) } catch (love::Exception &e) { - printf("Failed creating Pinned Memory StreamBuffer: %s\n", e.what()); + // According to the spec, oinned memory can fail if the RAM + // allocation can't be mapped to the GPU's address space. + // This seems to happen in practice on Mesa + amdgpu: + // https://bitbucket.org/rude/love/issues/1540 + // Fall through to other implementations when that happens. } } From 15f0e4109b3375a35dfe1cbf4723554483c93d62 Mon Sep 17 00:00:00 2001 From: Alex Szpakowski Date: Sat, 15 Feb 2020 13:50:47 -0400 Subject: [PATCH 3/3] Fix a typo in a comment --- src/modules/graphics/opengl/StreamBuffer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/graphics/opengl/StreamBuffer.cpp b/src/modules/graphics/opengl/StreamBuffer.cpp index 8ea3a39a8..10eed951e 100644 --- a/src/modules/graphics/opengl/StreamBuffer.cpp +++ b/src/modules/graphics/opengl/StreamBuffer.cpp @@ -505,9 +505,9 @@ love::graphics::StreamBuffer *CreateStreamBuffer(BufferType mode, size_t size) { return new StreamBufferPinnedMemory(mode, size); } - catch (love::Exception &e) + catch (love::Exception &) { - // According to the spec, oinned memory can fail if the RAM + // According to the spec, pinned memory can fail if the RAM // allocation can't be mapped to the GPU's address space. // This seems to happen in practice on Mesa + amdgpu: // https://bitbucket.org/rude/love/issues/1540