Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick d27d9d059b51 from angle #34045

Merged
merged 3 commits into from May 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/angle/.patches
Expand Up @@ -12,4 +12,5 @@ m99_vulkan_prevent_out_of_bounds_read_in_divisor_emulation_path.patch
m99_vulkan_streamvertexdatawithdivisor_write_beyond_buffer_boundary.patch
m98_protect_against_deleting_a_current_xfb_buffer.patch
m96-lts_vulkan_fix_issue_with_redefining_a_layered_attachment.patch
cherry-pick-d27d9d059b51.patch
m100_fix_crash_when_pausing_xfb_then_deleting_a_buffer.patch
100 changes: 100 additions & 0 deletions patches/angle/cherry-pick-d27d9d059b51.patch
@@ -0,0 +1,100 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Charlie Lao <cclao@google.com>
Date: Tue, 15 Mar 2022 09:39:36 -0700
Subject: Vulkan: Update mCurrentElementArrayBuffersync based on dirty bit

M96 merge issues:
ContextVk.cpp:
ContextVk::setupIndexedDraw: vertexArrayVk/getVertexArray() isn't present in M96
ContextVk::syncState: M96 uses mVertexArray instead of vertexArrayVk
VertexArrayVk.cpp:
VertexArrayVk::updateCurrentElementArrayBuffer doesn't exist in M9
Created it and kept M96 logic for retrieving buffer/offset

The previous fix crrev.com/c/3513553 has run into corner case that
requires more follow up change crrev.com/c/3522565. But with that, there
is report that now we are hitting assertion in
handleDirtyGraphicsIndexBuffer(). This becomes a bit fragile This new
fix relies on the DIRTY_BIT_INDEX_BUFFER dirty bit and should be more
reliable as long as the dirty bit is set properly (if not, then we have
other bug that it won't even send down vulkan command to bind the
correct element buffer). We could further optimize the code path and
create a fast path for most common usages in the future.

Bug: chromium:1299261
Change-Id: Ifa8f86d431798c9ca4c128ed71a3e9e0a3537ccb
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3526021
Commit-Queue: Charlie Lao <cclao@google.com>
(cherry picked from commit 349636a05a3577a127adb6c79a1e947890bbe462)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3605834
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Charlie Lao <cclao@google.com>

diff --git a/src/libANGLE/renderer/vulkan/ContextVk.cpp b/src/libANGLE/renderer/vulkan/ContextVk.cpp
index 15243f0c5a42949c2ce94dbd415089b77a7e39bc..2ff2fd75a73b50e3bce5d72e64d5d3064f30bd66 100644
--- a/src/libANGLE/renderer/vulkan/ContextVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ContextVk.cpp
@@ -954,6 +954,17 @@ angle::Result ContextVk::setupIndexedDraw(const gl::Context *context,
mGraphicsDirtyBits.set(DIRTY_BIT_INDEX_BUFFER);
mLastIndexBufferOffset = indices;
}
+
+ // When you draw with LineLoop mode or GL_UNSIGNED_BYTE type, we may allocate its own
+ // element buffer and modify mCurrentElementArrayBuffer. When we switch out of that draw
+ // mode, we must reset mCurrentElementArrayBuffer back to the vertexArray's element buffer.
+ // Since in either case we set DIRTY_BIT_INDEX_BUFFER dirty bit, we use this bit to re-sync
+ // mCurrentElementArrayBuffer.
+ if (mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER])
+ {
+ mVertexArray->updateCurrentElementArrayBuffer();
+ }
+
if (shouldConvertUint8VkIndexType(indexType) && mGraphicsDirtyBits[DIRTY_BIT_INDEX_BUFFER])
{
ANGLE_PERF_WARNING(getDebug(), GL_DEBUG_SEVERITY_LOW,
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index 93378c4b24495872405fc06ea01e15254229ab63..035c80a5ba95492247bd87e4189de602ffb47da1 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -492,6 +492,17 @@ angle::Result VertexArrayVk::convertVertexBufferCPU(ContextVk *contextVk,
return angle::Result::Continue;
}

+void VertexArrayVk::updateCurrentElementArrayBuffer()
+{
+ ASSERT(mState.getElementArrayBuffer() != nullptr);
+ ASSERT(mState.getElementArrayBuffer()->getSize() > 0);
+ gl::Buffer *bufferGL = mState.getElementArrayBuffer();
+ BufferVk *bufferVk = vk::GetImpl(bufferGL);
+ mCurrentElementArrayBuffer =
+ &bufferVk->getBufferAndOffset(&mCurrentElementArrayBufferOffset);
+
+}
+
angle::Result VertexArrayVk::syncState(const gl::Context *context,
const gl::VertexArray::DirtyBits &dirtyBits,
gl::VertexArray::DirtyAttribBitsArray *attribBits,
@@ -516,9 +527,7 @@ angle::Result VertexArrayVk::syncState(const gl::Context *context,
{
// Note that just updating buffer data may still result in a new
// vk::BufferHelper allocation.
- BufferVk *bufferVk = vk::GetImpl(bufferGL);
- mCurrentElementArrayBuffer =
- &bufferVk->getBufferAndOffset(&mCurrentElementArrayBufferOffset);
+ updateCurrentElementArrayBuffer();
}
else
{
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.h b/src/libANGLE/renderer/vulkan/VertexArrayVk.h
index c198265bf8ba2017a13fce558826862f450218b5..0b98a9ed46b7cd4b9588973c74b0bbaf9172ab6c 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.h
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.h
@@ -34,6 +34,8 @@ class VertexArrayVk : public VertexArrayImpl

angle::Result updateActiveAttribInfo(ContextVk *contextVk);

+ void updateCurrentElementArrayBuffer();
+
angle::Result updateDefaultAttrib(ContextVk *contextVk,
size_t attribIndex,
VkBuffer bufferHandle,