Commit 4756d5e6 by Tim Van Patten Committed by Commit Bot

Mark most draw modes valid when no program is bound

gpu_angle_passthrough_fuzzer discovered a crash when glDrawArrays() is called without a program bound. This was caused by assuming it's an error to draw certain primitive types without a program bound, which is incorrect. This fix is to mark all draw modes except Patches valid when there is no program bound. Patches is handled separately by TS validation. Bug: chromium:1185267 Bug: angleproject:5483 Test: gpu_angle_passthrough_fuzzer Test: SimpleOperationTest31.DrawWithoutProgramBound Test: KHR-GLES31.core.draw_indirect.basic-mode-*adjacency Change-Id: I294078b8695e0b8f36d3b7ad3c1aa71d2a275038 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2780971Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Tim Van Patten <timvp@google.com>
parent 5ee3826e
...@@ -252,18 +252,6 @@ void GetObjectLabelBase(const std::string &objectLabel, ...@@ -252,18 +252,6 @@ void GetObjectLabelBase(const std::string &objectLabel,
} }
} }
// The rest default to false.
constexpr angle::PackedEnumMap<PrimitiveMode, bool, angle::EnumSize<PrimitiveMode>() + 1>
kValidBasicDrawModes = {{
{PrimitiveMode::Points, true},
{PrimitiveMode::Lines, true},
{PrimitiveMode::LineLoop, true},
{PrimitiveMode::LineStrip, true},
{PrimitiveMode::Triangles, true},
{PrimitiveMode::TriangleStrip, true},
{PrimitiveMode::TriangleFan, true},
}};
enum SubjectIndexes : angle::SubjectIndex enum SubjectIndexes : angle::SubjectIndex
{ {
kTexture0SubjectIndex = 0, kTexture0SubjectIndex = 0,
...@@ -9047,7 +9035,9 @@ StateCache::StateCache() ...@@ -9047,7 +9035,9 @@ StateCache::StateCache()
mCachedBasicDrawElementsError(kInvalidPointer), mCachedBasicDrawElementsError(kInvalidPointer),
mCachedTransformFeedbackActiveUnpaused(false), mCachedTransformFeedbackActiveUnpaused(false),
mCachedCanDraw(false) mCachedCanDraw(false)
{} {
mCachedValidDrawModes.fill(false);
}
StateCache::~StateCache() = default; StateCache::~StateCache() = default;
...@@ -9276,11 +9266,11 @@ void StateCache::setValidDrawModes(bool pointsOK, ...@@ -9276,11 +9266,11 @@ void StateCache::setValidDrawModes(bool pointsOK,
{ {
mCachedValidDrawModes[PrimitiveMode::Points] = pointsOK; mCachedValidDrawModes[PrimitiveMode::Points] = pointsOK;
mCachedValidDrawModes[PrimitiveMode::Lines] = linesOK; mCachedValidDrawModes[PrimitiveMode::Lines] = linesOK;
mCachedValidDrawModes[PrimitiveMode::LineStrip] = linesOK;
mCachedValidDrawModes[PrimitiveMode::LineLoop] = linesOK; mCachedValidDrawModes[PrimitiveMode::LineLoop] = linesOK;
mCachedValidDrawModes[PrimitiveMode::LineStrip] = linesOK;
mCachedValidDrawModes[PrimitiveMode::Triangles] = trisOK; mCachedValidDrawModes[PrimitiveMode::Triangles] = trisOK;
mCachedValidDrawModes[PrimitiveMode::TriangleFan] = trisOK;
mCachedValidDrawModes[PrimitiveMode::TriangleStrip] = trisOK; mCachedValidDrawModes[PrimitiveMode::TriangleStrip] = trisOK;
mCachedValidDrawModes[PrimitiveMode::TriangleFan] = trisOK;
mCachedValidDrawModes[PrimitiveMode::LinesAdjacency] = lineAdjOK; mCachedValidDrawModes[PrimitiveMode::LinesAdjacency] = lineAdjOK;
mCachedValidDrawModes[PrimitiveMode::LineStripAdjacency] = lineAdjOK; mCachedValidDrawModes[PrimitiveMode::LineStripAdjacency] = lineAdjOK;
mCachedValidDrawModes[PrimitiveMode::TrianglesAdjacency] = triAdjOK; mCachedValidDrawModes[PrimitiveMode::TrianglesAdjacency] = triAdjOK;
...@@ -9324,18 +9314,19 @@ void StateCache::updateValidDrawModes(Context *context) ...@@ -9324,18 +9314,19 @@ void StateCache::updateValidDrawModes(Context *context)
if (!programExecutable || !programExecutable->hasLinkedShaderStage(ShaderType::Geometry)) if (!programExecutable || !programExecutable->hasLinkedShaderStage(ShaderType::Geometry))
{ {
mCachedValidDrawModes = kValidBasicDrawModes; // All draw modes are valid, since drawing without a program does not generate an error and
// and operations requiring a GS will trigger other validation errors.
// `patchOK = false` due to checking above already enabling it if a TS is present.
setValidDrawModes(true, true, true, true, true, false);
return; return;
} }
ASSERT(programExecutable->hasLinkedShaderStage(ShaderType::Geometry));
PrimitiveMode gsMode = programExecutable->getGeometryShaderInputPrimitiveType(); PrimitiveMode gsMode = programExecutable->getGeometryShaderInputPrimitiveType();
bool pointsOK = gsMode == PrimitiveMode::Points;
bool pointsOK = gsMode == PrimitiveMode::Points; bool linesOK = gsMode == PrimitiveMode::Lines;
bool linesOK = gsMode == PrimitiveMode::Lines; bool trisOK = gsMode == PrimitiveMode::Triangles;
bool trisOK = gsMode == PrimitiveMode::Triangles; bool lineAdjOK = gsMode == PrimitiveMode::LinesAdjacency;
bool lineAdjOK = gsMode == PrimitiveMode::LinesAdjacency; bool triAdjOK = gsMode == PrimitiveMode::TrianglesAdjacency;
bool triAdjOK = gsMode == PrimitiveMode::TrianglesAdjacency;
setValidDrawModes(pointsOK, linesOK, trisOK, lineAdjOK, triAdjOK, false); setValidDrawModes(pointsOK, linesOK, trisOK, lineAdjOK, triAdjOK, false);
} }
......
...@@ -62,9 +62,6 @@ ...@@ -62,9 +62,6 @@
4723 VULKAN NVIDIA : KHR-GLES31.core.shader_image_size.advanced-nonMS-vs-uint = SKIP 4723 VULKAN NVIDIA : KHR-GLES31.core.shader_image_size.advanced-nonMS-vs-uint = SKIP
4723 VULKAN NVIDIA : KHR-GLES31.core.program_interface_query.transform-feedback-types = SKIP 4723 VULKAN NVIDIA : KHR-GLES31.core.program_interface_query.transform-feedback-types = SKIP
// Geometry shader support
5483 VULKAN : KHR-GLES31.core.draw_indirect.basic-mode-*adjacency = SKIP
// New failures with latest dEQP roll (2021-03-05) // New failures with latest dEQP roll (2021-03-05)
5722 VULKAN : KHR-GLES31.core.framebuffer_completeness.rbo_and_texture_expect_zero_numsamples = FAIL 5722 VULKAN : KHR-GLES31.core.framebuffer_completeness.rbo_and_texture_expect_zero_numsamples = FAIL
......
...@@ -52,6 +52,9 @@ class SimpleOperationTest : public ANGLETest ...@@ -52,6 +52,9 @@ class SimpleOperationTest : public ANGLETest
int windowHeight); int windowHeight);
}; };
class SimpleOperationTest31 : public SimpleOperationTest
{};
void SimpleOperationTest::verifyBuffer(const std::vector<uint8_t> &data, GLenum binding) void SimpleOperationTest::verifyBuffer(const std::vector<uint8_t> &data, GLenum binding)
{ {
if (!IsGLExtensionEnabled("GL_EXT_map_buffer_range")) if (!IsGLExtensionEnabled("GL_EXT_map_buffer_range"))
...@@ -1205,6 +1208,21 @@ TEST_P(SimpleOperationTest, PrimitiveModeNegativeTest) ...@@ -1205,6 +1208,21 @@ TEST_P(SimpleOperationTest, PrimitiveModeNegativeTest)
EXPECT_GL_ERROR(GL_INVALID_ENUM); EXPECT_GL_ERROR(GL_INVALID_ENUM);
} }
// Verify we don't crash when attempting to draw using GL_TRIANGLES without a program bound.
TEST_P(SimpleOperationTest31, DrawTrianglesWithoutProgramBound)
{
glDrawArrays(GL_TRIANGLES, 0, 6);
}
// Verify we don't crash when attempting to draw using GL_LINE_STRIP_ADJACENCY without a program
// bound.
TEST_P(SimpleOperationTest31, DrawLineStripAdjacencyWithoutProgramBound)
{
ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_EXT_geometry_shader"));
glDrawArrays(GL_LINE_STRIP_ADJACENCY, 0, 10);
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these // Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against. // tests should be run against.
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND( ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND(
...@@ -1223,4 +1241,6 @@ ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND( ...@@ -1223,4 +1241,6 @@ ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND(
/* cheapRenderPass */ false), /* cheapRenderPass */ false),
WithNoVulkanViewportFlip(ES2_VULKAN())); WithNoVulkanViewportFlip(ES2_VULKAN()));
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND_ES31(SimpleOperationTest31);
} // namespace } // namespace
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