Commit 1eda27a6 by Jamie Madill Committed by Commit Bot

Buffer11: Allow CopySubData from uninitialized.

This fixes a very odd use case where an app would try to copy from an uninitialized buffer. I didn't search the spec too closely, but it's likely a valid operation that produces undefined buffer contents. Previously to this change we would genearte an OOM error. Also includes an unrelated fix to ensure the latest buffer storage is never nullptr when we have any data. Bug: angleproject:1155 Change-Id: I4292bd302cc2b84d125a7d3e8d28e4d2b0210e53 Reviewed-on: https://chromium-review.googlesource.com/774991Reviewed-by: 's avatarYuly Novikov <ynovikov@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent 78413499
...@@ -432,11 +432,13 @@ gl::Error Buffer11::copySubData(const gl::Context *context, ...@@ -432,11 +432,13 @@ gl::Error Buffer11::copySubData(const gl::Context *context,
BufferStorage *copySource = nullptr; BufferStorage *copySource = nullptr;
ANGLE_TRY_RESULT(sourceBuffer->getLatestBufferStorage(context), copySource); ANGLE_TRY_RESULT(sourceBuffer->getLatestBufferStorage(context), copySource);
if (!copySource || !copyDest) if (!copySource)
{ {
return gl::OutOfMemory() << "Failed to allocate internal staging buffer."; ANGLE_TRY_RESULT(sourceBuffer->getStagingStorage(context), copySource);
} }
ASSERT(copySource && copyDest);
// A staging buffer is needed if there is no cpu-cpu or gpu-gpu copy path avaiable. // A staging buffer is needed if there is no cpu-cpu or gpu-gpu copy path avaiable.
if (!copyDest->isGPUAccessible() && !copySource->isCPUAccessible(GL_MAP_READ_BIT)) if (!copyDest->isGPUAccessible() && !copySource->isCPUAccessible(GL_MAP_READ_BIT))
{ {
...@@ -831,40 +833,46 @@ gl::Error Buffer11::updateBufferStorage(const gl::Context *context, ...@@ -831,40 +833,46 @@ gl::Error Buffer11::updateBufferStorage(const gl::Context *context,
ASSERT(storage); ASSERT(storage);
if (latestBuffer && latestBuffer->getDataRevision() > storage->getDataRevision()) if (!latestBuffer)
{ {
// Copy through a staging buffer if we're copying from or to a non-staging, mappable onStorageUpdate(storage);
// buffer storage. This is because we can't map a GPU buffer, and copy CPU return gl::NoError();
// data directly. If we're already using a staging buffer we're fine. }
if (latestBuffer->getUsage() != BUFFER_USAGE_STAGING &&
storage->getUsage() != BUFFER_USAGE_STAGING &&
(!latestBuffer->isCPUAccessible(GL_MAP_READ_BIT) ||
!storage->isCPUAccessible(GL_MAP_WRITE_BIT)))
{
NativeStorage *stagingBuffer = nullptr;
ANGLE_TRY_RESULT(getStagingStorage(context), stagingBuffer);
CopyResult copyResult = CopyResult::NOT_RECREATED; if (latestBuffer->getDataRevision() <= storage->getDataRevision())
ANGLE_TRY_RESULT(stagingBuffer->copyFromStorage(context, latestBuffer, 0, {
latestBuffer->getSize(), 0), return gl::NoError();
copyResult); }
onCopyStorage(stagingBuffer, latestBuffer);
latestBuffer = stagingBuffer; // Copy through a staging buffer if we're copying from or to a non-staging, mappable
} // buffer storage. This is because we can't map a GPU buffer, and copy CPU
// data directly. If we're already using a staging buffer we're fine.
if (latestBuffer->getUsage() != BUFFER_USAGE_STAGING &&
storage->getUsage() != BUFFER_USAGE_STAGING &&
(!latestBuffer->isCPUAccessible(GL_MAP_READ_BIT) ||
!storage->isCPUAccessible(GL_MAP_WRITE_BIT)))
{
NativeStorage *stagingBuffer = nullptr;
ANGLE_TRY_RESULT(getStagingStorage(context), stagingBuffer);
CopyResult copyResult = CopyResult::NOT_RECREATED; CopyResult copyResult = CopyResult::NOT_RECREATED;
ANGLE_TRY_RESULT( ANGLE_TRY_RESULT(
storage->copyFromStorage(context, latestBuffer, sourceOffset, storageSize, 0), stagingBuffer->copyFromStorage(context, latestBuffer, 0, latestBuffer->getSize(), 0),
copyResult); copyResult);
// If the D3D buffer has been recreated, we should update our serial. onCopyStorage(stagingBuffer, latestBuffer);
if (copyResult == CopyResult::RECREATED)
{ latestBuffer = stagingBuffer;
updateSerial();
}
onCopyStorage(storage, latestBuffer);
} }
CopyResult copyResult = CopyResult::NOT_RECREATED;
ANGLE_TRY_RESULT(storage->copyFromStorage(context, latestBuffer, sourceOffset, storageSize, 0),
copyResult);
// If the D3D buffer has been recreated, we should update our serial.
if (copyResult == CopyResult::RECREATED)
{
updateSerial();
}
onCopyStorage(storage, latestBuffer);
return gl::NoError(); return gl::NoError();
} }
......
...@@ -404,6 +404,23 @@ TEST_P(BufferDataTest, MapBufferOES) ...@@ -404,6 +404,23 @@ TEST_P(BufferDataTest, MapBufferOES)
EXPECT_EQ(data, actualData); EXPECT_EQ(data, actualData);
} }
// Tests a bug where copying buffer data immediately after creation hit a nullptr in D3D11.
TEST_P(BufferDataTestES3, NoBufferInitDataCopyBug)
{
constexpr GLsizei size = 64;
GLBuffer sourceBuffer;
glBindBuffer(GL_COPY_READ_BUFFER, sourceBuffer);
glBufferData(GL_COPY_READ_BUFFER, size, nullptr, GL_STATIC_DRAW);
GLBuffer destBuffer;
glBindBuffer(GL_ARRAY_BUFFER, destBuffer);
glBufferData(GL_ARRAY_BUFFER, size, nullptr, GL_STATIC_DRAW);
glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_ARRAY_BUFFER, 0, 0, size);
ASSERT_GL_NO_ERROR();
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against. // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
ANGLE_INSTANTIATE_TEST(BufferDataTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL(), ES2_OPENGLES()); ANGLE_INSTANTIATE_TEST(BufferDataTest, ES2_D3D9(), ES2_D3D11(), ES2_OPENGL(), ES2_OPENGLES());
ANGLE_INSTANTIATE_TEST(BufferDataTestES3, ES3_D3D11(), ES3_OPENGL(), ES3_OPENGLES()); ANGLE_INSTANTIATE_TEST(BufferDataTestES3, ES3_D3D11(), ES3_OPENGL(), ES3_OPENGLES());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment