-
Notifications
You must be signed in to change notification settings - Fork 15k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: [19-x-y] cherry-pick 1121a459f094 from angle
- Loading branch information
Showing
3 changed files
with
126 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
m104_vulkan_fix_xfb_buffer_redefine_to_smaller_size.patch |
122 changes: 122 additions & 0 deletions
122
patches/angle/m104_vulkan_fix_xfb_buffer_redefine_to_smaller_size.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,122 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Shahbaz Youssefi <syoussefi@chromium.org> | ||
Date: Tue, 26 Jul 2022 21:07:04 -0400 | ||
Subject: M104: Vulkan: Fix xfb buffer redefine to smaller size | ||
|
||
In 89e11878b275b15735eaf273ababfa6fd43a2e3d, a use-after-free bug was | ||
fixed where glBufferData redefined a buffer, leading to a change in | ||
storage. This was only tested for the case where the new buffer was | ||
larger than the old buffer. | ||
|
||
When the new buffer is smaller however, another issue remains where the | ||
buffer size as cached by the transform feedback object used the old | ||
object's size. This is worked around in this change, with a fix for the | ||
real issue (that the buffer state is updated after calling into the | ||
backend instead of before) coming up. | ||
|
||
Bug: chromium:1345042 | ||
Change-Id: I7bafd51b6203a419e5ef123da26b9e1eaf079bf1 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3812556 | ||
Reviewed-by: Ian Elliott <ianelliott@google.com> | ||
|
||
diff --git a/src/libANGLE/renderer/vulkan/BufferVk.cpp b/src/libANGLE/renderer/vulkan/BufferVk.cpp | ||
index 9c9cee78d890f5cc13d8762aa03de9dcf9c00abf..ceb065822984cf44bd4319fb97cb7e4dd49517b2 100644 | ||
--- a/src/libANGLE/renderer/vulkan/BufferVk.cpp | ||
+++ b/src/libANGLE/renderer/vulkan/BufferVk.cpp | ||
@@ -798,6 +798,7 @@ angle::Result BufferVk::updateBuffer(ContextVk *contextVk, | ||
} | ||
return angle::Result::Continue; | ||
} | ||
+ | ||
angle::Result BufferVk::directUpdate(ContextVk *contextVk, | ||
const uint8_t *data, | ||
size_t size, | ||
diff --git a/src/libANGLE/renderer/vulkan/ContextVk.cpp b/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
index 90ecf42b2cc22cee5b74296d7a2f73b77f9a0f2c..f733ac42186606f38698660bd73b05862da48993 100644 | ||
--- a/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
+++ b/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
@@ -799,7 +799,8 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk | ||
DIRTY_BIT_DRIVER_UNIFORMS_BINDING, | ||
DIRTY_BIT_VIEWPORT, | ||
DIRTY_BIT_SCISSOR}; | ||
- if (getFeatures().supportsTransformFeedbackExtension.enabled) | ||
+ if (getFeatures().supportsTransformFeedbackExtension.enabled || | ||
+ getFeatures().emulateTransformFeedback.enabled) | ||
{ | ||
mNewGraphicsCommandBufferDirtyBits.set(DIRTY_BIT_TRANSFORM_FEEDBACK_BUFFERS); | ||
} | ||
diff --git a/src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp b/src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp | ||
index dbaa4eeedbf36c4a3f6cb818b50bdcac856766c2..f785fe5f7c36033163a77a7e29957ebca6edaf79 100644 | ||
--- a/src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp | ||
+++ b/src/libANGLE/renderer/vulkan/TransformFeedbackVk.cpp | ||
@@ -362,7 +362,8 @@ void TransformFeedbackVk::onSubjectStateChange(angle::SubjectIndex index, | ||
ASSERT(bufferVk->isBufferValid()); | ||
mBufferHelpers[index] = &bufferVk->getBuffer(); | ||
mBufferOffsets[index] = binding.getOffset() + mBufferHelpers[index]->getOffset(); | ||
- mBufferSizes[index] = gl::GetBoundBufferAvailableSize(binding); | ||
+ mBufferSizes[index] = std::min<VkDeviceSize>(gl::GetBoundBufferAvailableSize(binding), | ||
+ mBufferHelpers[index]->getSize()); | ||
mBufferObserverBindings[index].bind(bufferVk); | ||
|
||
mXFBBuffersDesc.updateTransformFeedbackBuffer( | ||
diff --git a/src/tests/angle_end2end_tests_expectations.txt b/src/tests/angle_end2end_tests_expectations.txt | ||
index af74f258fb3d901dac8a738ca9be53511963137b..22230db0ecb77f16cb2cc494a74225731401a47d 100644 | ||
--- a/src/tests/angle_end2end_tests_expectations.txt | ||
+++ b/src/tests/angle_end2end_tests_expectations.txt | ||
@@ -161,6 +161,7 @@ | ||
6738 MAC AMD OPENGL : Texture3DTestES3.PixelUnpackStateTex* = SKIP | ||
1296467 MAC OPENGL : VertexAttributeTestES3.emptyBuffer/* = SKIP | ||
7203 MAC INTEL OPENGL : CopyTextureTest.CopyToMipmap/* = SKIP | ||
+7530 MAC NVIDIA OPENGL : TransformFeedbackTest.RenderOnceChangeXfbBufferRenderAgain/* = SKIP | ||
|
||
// BlitFramebufferTest.ScissoredMultisampleStencil failures | ||
3496 MAC INTEL OPENGL : BlitFramebufferTest.ScissoredMultisampleStencil/* = SKIP | ||
diff --git a/src/tests/gl_tests/TransformFeedbackTest.cpp b/src/tests/gl_tests/TransformFeedbackTest.cpp | ||
index a4e4f86157232de46c6e06ee7b5a6dfa2b8a2261..dc944d4249e0a26708d01bd0bde7efc4d68db3e3 100644 | ||
--- a/src/tests/gl_tests/TransformFeedbackTest.cpp | ||
+++ b/src/tests/gl_tests/TransformFeedbackTest.cpp | ||
@@ -403,7 +403,6 @@ TEST_P(TransformFeedbackTest, RecordAndDraw) | ||
// Test that transform feedback can cover multiple render passes. | ||
TEST_P(TransformFeedbackTest, SpanMultipleRenderPasses) | ||
{ | ||
- | ||
// TODO(anglebug.com/4533) This fails after the upgrade to the 26.20.100.7870 driver. | ||
ANGLE_SKIP_TEST_IF(IsWindows() && IsIntel() && IsVulkan()); | ||
|
||
@@ -4105,6 +4104,36 @@ TEST_P(TransformFeedbackTest, ResumingTransformFeedbackAfterDeletebuffer) | ||
ASSERT_GL_ERROR(GL_INVALID_OPERATION); | ||
} | ||
|
||
+// Test that redefining the transform feedback buffer and starting a new render pass works. | ||
+TEST_P(TransformFeedbackTest, RenderOnceChangeXfbBufferRenderAgain) | ||
+{ | ||
+ std::vector<std::string> tfVaryings; | ||
+ tfVaryings.push_back("gl_Position"); | ||
+ ANGLE_GL_PROGRAM_TRANSFORM_FEEDBACK(drawColor, essl3_shaders::vs::Simple(), | ||
+ essl3_shaders::fs::Red(), tfVaryings, | ||
+ GL_INTERLEAVED_ATTRIBS); | ||
+ | ||
+ GLBuffer buffer; | ||
+ glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, buffer); | ||
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, 10'000'000, nullptr, GL_DYNAMIC_READ); | ||
+ | ||
+ glUseProgram(drawColor); | ||
+ glBeginTransformFeedback(GL_TRIANGLES); | ||
+ | ||
+ drawQuad(drawColor, essl3_shaders::PositionAttrib(), 0.5f); | ||
+ | ||
+ // Break the render pass | ||
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::red); | ||
+ | ||
+ // Redefine the transform feedback buffer | ||
+ glBufferData(GL_TRANSFORM_FEEDBACK_BUFFER, 40, nullptr, GL_DYNAMIC_READ); | ||
+ | ||
+ // Start a new render pass | ||
+ drawQuad(drawColor, essl3_shaders::PositionAttrib(), 0.5f); | ||
+ | ||
+ glEndTransformFeedback(); | ||
+} | ||
+ | ||
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(TransformFeedbackTest); | ||
ANGLE_INSTANTIATE_TEST_ES3(TransformFeedbackTest); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters