Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 8 changes from Release-1-M123 (#41746)
* chore: cherry-pick 8 changes from Release-1-M123 * chore: update patches --------- Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
- Loading branch information
1 parent
8647232
commit ad9a90e
Showing
14 changed files
with
2,222 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 @@ | ||
m123_vulkan_fix_access_to_inactive_attributes.patch |
112 changes: 112 additions & 0 deletions
112
patches/angle/m123_vulkan_fix_access_to_inactive_attributes.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,112 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Geoff Lang <geofflang@chromium.org> | ||
Date: Tue, 12 Mar 2024 16:06:37 -0400 | ||
Subject: M123: Vulkan: Fix access to inactive attributes | ||
|
||
... within range of active ones. Since a buffer is bound for inactive | ||
attributes, it must be considered accessed. | ||
|
||
Ultimately, the nullDescriptor feature could be used to avoid binding a | ||
buffer for inactive attributes. | ||
|
||
Bug: chromium:327807820 | ||
Change-Id: I953b419d8ec51760e8848409024cad5083888fa2 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5386431 | ||
Reviewed-by: Shahbaz Youssefi <syoussefi@google.com> | ||
|
||
diff --git a/src/libANGLE/renderer/vulkan/ContextVk.cpp b/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
index 63bfa0729b266ceca54e10153f561f74a1be0c27..a0cbaf8cefbae1453922e09aadcd13df6f478782 100644 | ||
--- a/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
+++ b/src/libANGLE/renderer/vulkan/ContextVk.cpp | ||
@@ -2610,8 +2610,7 @@ angle::Result ContextVk::handleDirtyGraphicsVertexBuffers(DirtyBits::Iterator *d | ||
vertexArrayVk->getCurrentArrayBuffers(); | ||
|
||
// Mark all active vertex buffers as accessed. | ||
- const gl::AttributesMask attribsMask = executable->getActiveAttribLocationsMask(); | ||
- for (size_t attribIndex : attribsMask) | ||
+ for (uint32_t attribIndex = 0; attribIndex < maxAttrib; ++attribIndex) | ||
{ | ||
vk::BufferHelper *arrayBuffer = arrayBufferResources[attribIndex]; | ||
if (arrayBuffer) | ||
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp | ||
index b8a1c87728b3ba54a32cf0e4da6ca626c05d1d92..773bbf026821795c0db34239d27fd2bb1e5a751a 100644 | ||
--- a/src/tests/gl_tests/VertexAttributeTest.cpp | ||
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp | ||
@@ -1256,6 +1256,19 @@ class VertexAttributeOORTest : public VertexAttributeTest | ||
} | ||
}; | ||
|
||
+class RobustVertexAttributeTest : public VertexAttributeTest | ||
+{ | ||
+ public: | ||
+ RobustVertexAttributeTest() | ||
+ { | ||
+ // mac GL and metal do not support robustness. | ||
+ if (!IsMac() && !IsIOS()) | ||
+ { | ||
+ setRobustAccess(true); | ||
+ } | ||
+ } | ||
+}; | ||
+ | ||
// Verify that drawing with a large out-of-range offset generates INVALID_OPERATION. | ||
// Requires WebGL compatibility with robust access behaviour disabled. | ||
TEST_P(VertexAttributeOORTest, ANGLEDrawArraysBufferTooSmall) | ||
@@ -1316,6 +1329,48 @@ TEST_P(VertexAttributeOORTest, ANGLEDrawArraysOutOfBoundsCases) | ||
EXPECT_GL_ERROR(GL_INVALID_OPERATION); | ||
} | ||
|
||
+// Test that enabling a buffer in an unused attribute doesn't crash. There should be an active | ||
+// attribute after that. | ||
+TEST_P(RobustVertexAttributeTest, BoundButUnusedBuffer) | ||
+{ | ||
+ constexpr char kVS[] = R"(attribute vec2 offset; | ||
+void main() | ||
+{ | ||
+ gl_Position = vec4(offset.xy, 0, 1); | ||
+ gl_PointSize = 1.0; | ||
+})"; | ||
+ | ||
+ constexpr char kFS[] = R"(precision mediump float; | ||
+void main() | ||
+{ | ||
+ gl_FragColor = vec4(1.0, 0, 0, 1.0); | ||
+})"; | ||
+ | ||
+ const GLuint vs = CompileShader(GL_VERTEX_SHADER, kVS); | ||
+ const GLuint fs = CompileShader(GL_FRAGMENT_SHADER, kFS); | ||
+ | ||
+ GLuint program = glCreateProgram(); | ||
+ glBindAttribLocation(program, 1, "offset"); | ||
+ glAttachShader(program, vs); | ||
+ glAttachShader(program, fs); | ||
+ glLinkProgram(program); | ||
+ | ||
+ GLBuffer buffer; | ||
+ glBindBuffer(GL_ARRAY_BUFFER, buffer); | ||
+ glBufferData(GL_ARRAY_BUFFER, 100, nullptr, GL_STATIC_DRAW); | ||
+ | ||
+ // Enable an unused attribute that is within the range of active attributes (not beyond it) | ||
+ glEnableVertexAttribArray(0); | ||
+ glVertexAttribPointer(0, 4, GL_FLOAT, false, 0, 0); | ||
+ | ||
+ glUseProgram(program); | ||
+ glDrawArrays(GL_TRIANGLES, 0, 6); | ||
+ | ||
+ // Destroy the buffer. Regression test for a tracking bug where the buffer was used by | ||
+ // SwiftShader (even though location 1 is inactive), but not marked as used by ANGLE. | ||
+ buffer.reset(); | ||
+} | ||
+ | ||
// Verify that using a different start vertex doesn't mess up the draw. | ||
TEST_P(VertexAttributeTest, DrawArraysWithBufferOffset) | ||
{ | ||
@@ -4913,6 +4968,8 @@ ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND( | ||
ES3_METAL().disable(Feature::HasExplicitMemBarrier).disable(Feature::HasCheapRenderPass), | ||
ES3_METAL().disable(Feature::HasExplicitMemBarrier).enable(Feature::HasCheapRenderPass)); | ||
|
||
+ANGLE_INSTANTIATE_TEST_ES2_AND_ES3(RobustVertexAttributeTest); | ||
+ | ||
GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(VertexAttributeTestES3); | ||
ANGLE_INSTANTIATE_TEST_ES3_AND( | ||
VertexAttributeTestES3, |
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
36 changes: 36 additions & 0 deletions
36
patches/chromium/fix_paintimage_deserialization_arbitrary-read_issue.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,36 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Peng Huang <penghuang@chromium.org> | ||
Date: Wed, 20 Mar 2024 16:22:16 +0000 | ||
Subject: Fix PaintImage deserialization arbitrary-read issue | ||
|
||
(cherry picked from commit 47e8386c97ac7a84a96866fbd35422b99a01de5a) | ||
|
||
Bug: 327183408 | ||
Change-Id: I09927fbae60b666aaa370e3aba01607cdb977a25 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5370455 | ||
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> | ||
Commit-Queue: Peng Huang <penghuang@chromium.org> | ||
Cr-Original-Commit-Position: refs/heads/main@{#1272930} | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5382202 | ||
Auto-Submit: Peng Huang <penghuang@chromium.org> | ||
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> | ||
Cr-Commit-Position: refs/branch-heads/6261@{#1106} | ||
Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580} | ||
|
||
diff --git a/cc/paint/paint_op_reader.cc b/cc/paint/paint_op_reader.cc | ||
index 22a044734c898997d13f34a04b10e356cc86717e..46c385054b1575cff7ad2ae38be237deea081914 100644 | ||
--- a/cc/paint/paint_op_reader.cc | ||
+++ b/cc/paint/paint_op_reader.cc | ||
@@ -1572,9 +1572,10 @@ inline void PaintOpReader::DidRead(size_t bytes_read) { | ||
// All data are aligned with PaintOpWriter::kDefaultAlignment at least. | ||
size_t aligned_bytes = | ||
base::bits::AlignUp(bytes_read, PaintOpWriter::kDefaultAlignment); | ||
- memory_ += aligned_bytes; | ||
DCHECK_LE(aligned_bytes, remaining_bytes_); | ||
- remaining_bytes_ -= aligned_bytes; | ||
+ bytes_read = std::min(aligned_bytes, remaining_bytes_); | ||
+ memory_ += bytes_read; | ||
+ remaining_bytes_ -= bytes_read; | ||
} | ||
|
||
} // namespace cc |
80 changes: 80 additions & 0 deletions
80
patches/chromium/m122_webcodecs_disable_async_videoframe_readback_to_mitigate_a.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,80 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Eugene Zemtsov <eugene@chromium.org> | ||
Date: Mon, 25 Mar 2024 19:28:44 +0000 | ||
Subject: webcodecs: Disable async VideoFrame readback to mitigate a race | ||
|
||
(cherry picked from commit fdc363eb7a1c1c194a02a4cb340534b1501b0f95) | ||
|
||
Bug: 330575496 | ||
Change-Id: I187a113528da9d1c4316186e3dd24f91dbfd818b | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5386784 | ||
Commit-Queue: Eugene Zemtsov <eugene@chromium.org> | ||
Reviewed-by: Dale Curtis <dalecurtis@chromium.org> | ||
Cr-Original-Commit-Position: refs/heads/main@{#1277172} | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5391828 | ||
Reviewed-by: Eugene Zemtsov <eugene@chromium.org> | ||
Commit-Queue: Dale Curtis <dalecurtis@chromium.org> | ||
Reviewed-by: Xiaohan Wang <xhwang@chromium.org> | ||
Commit-Queue: Xiaohan Wang <xhwang@chromium.org> | ||
Auto-Submit: Dale Curtis <dalecurtis@chromium.org> | ||
Cr-Commit-Position: refs/branch-heads/6261@{#1124} | ||
Cr-Branched-From: 9755d9d81e4a8cb5b4f76b23b761457479dbb06b-refs/heads/main@{#1250580} | ||
|
||
diff --git a/content/test/data/gpu/webcodecs/copyTo.html b/content/test/data/gpu/webcodecs/copyTo.html | ||
index ec2455c9c18900ad911ce98f326139cbdeabd84f..9453c8d361a572b500e86b1249896bc4114ebe27 100644 | ||
--- a/content/test/data/gpu/webcodecs/copyTo.html | ||
+++ b/content/test/data/gpu/webcodecs/copyTo.html | ||
@@ -118,6 +118,16 @@ Take frames coming from various sources and read them using copyTo(). | ||
let frame = await source.getNextFrame(); | ||
let size = frame.allocationSize(); | ||
|
||
+ // Readback a whole frame to a regular buffer detach it | ||
+ { | ||
+ let buf = new ArrayBuffer(size); | ||
+ TEST.assert(readWholeBuffer(buf) == 0, "Buffer should be zero"); | ||
+ let copy_promise = frame.copyTo(buf); | ||
+ buf.transfer(1); | ||
+ let layout = await copy_promise; | ||
+ TEST.assert(layout, "layout is empty / ArrayBuffer"); | ||
+ } | ||
+ | ||
// Readback a whole frame to a regular buffer and send it to a worker | ||
{ | ||
let {worker, worker_promise } = makeWorker(); | ||
@@ -158,4 +168,5 @@ Take frames coming from various sources and read them using copyTo(). | ||
TEST.log('Test completed'); | ||
} | ||
addManualTestButton([{'source_type': 'offscreen'}]); | ||
+ addManualTestButton([{'source_type': 'arraybuffer'}]); | ||
</script> | ||
diff --git a/third_party/blink/renderer/modules/webcodecs/video_frame.cc b/third_party/blink/renderer/modules/webcodecs/video_frame.cc | ||
index 279359ea2d536358ce946e6f7d8feec2dfcc160c..e37dd9568399283f8006dfd1578c0e5b57566830 100644 | ||
--- a/third_party/blink/renderer/modules/webcodecs/video_frame.cc | ||
+++ b/third_party/blink/renderer/modules/webcodecs/video_frame.cc | ||
@@ -80,6 +80,11 @@ namespace blink { | ||
|
||
namespace { | ||
|
||
+// Controls if VideoFrame.copyTo() reads GPU frames asynchronously | ||
+BASE_FEATURE(kVideoFrameAsyncCopyTo, | ||
+ "VideoFrameAsyncCopyTo", | ||
+ base::FEATURE_DISABLED_BY_DEFAULT); | ||
+ | ||
media::VideoPixelFormat ToMediaPixelFormat(V8VideoPixelFormat::Enum fmt) { | ||
switch (fmt) { | ||
case V8VideoPixelFormat::Enum::kI420: | ||
@@ -1217,9 +1222,11 @@ ScriptPromise VideoFrame::copyTo(ScriptState* script_state, | ||
} else { | ||
DCHECK(local_frame->HasTextures()); | ||
|
||
- if (auto* resolver = CopyToAsync(script_state, local_frame, src_rect, | ||
- destination, dest_layout)) { | ||
- return resolver->Promise(); | ||
+ if (base::FeatureList::IsEnabled(kVideoFrameAsyncCopyTo)) { | ||
+ if (auto* resolver = CopyToAsync(script_state, local_frame, src_rect, | ||
+ destination, dest_layout)) { | ||
+ return resolver->Promise(); | ||
+ } | ||
} | ||
|
||
if (!CopyTexturablePlanes(*local_frame, src_rect, dest_layout, buffer)) { |
Oops, something went wrong.