Validate program changes wrt transform feedback
UseProgram can't be called while transform feedback is active and
unpaused. Validate this by checking the presence of active transform
feedback in UseProgram.
LinkProgram or ProgramBinary can't be called while transform feedback
associated with the program is active. Validate this by going through
all of the existing transform feedback objects when one of these
functions is called and checking whether they are associated with the
program being changed. A program association is added to
gl::TransformFeedback to facilitate this.
BeginTransformFeedback can't be used to unpause a transform feedback
object, so code for that is removed.
The validation of the entry points touched in this patch is refactored
to follow the current convention of separate Validate* functions,
though with LinkProgram following this convention fully isn't
practical.
This patch also makes sure that ANGLE doesn't invoke behavior that the
GL spec doesn't specify if a program object associated with a paused
transform feedback is deleted.
Tests are edited so that they don't call UseProgram when it generates
an error.
BUG=angleproject:1101
TEST=dEQP-GLES3.functional.negative_api.shader.* (2 more tests pass),
dEQP-GLES3.functional.transform_feedback.* (no regressions),
angle_end2end_tests
Change-Id: I2e5b3a027ced11249b762ec01a29fa41d2c0dd96
Reviewed-on: https://chromium-review.googlesource.com/332141
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/libANGLE/Context.cpp b/src/libANGLE/Context.cpp
index dba4aa9..6dad59f 100644
--- a/src/libANGLE/Context.cpp
+++ b/src/libANGLE/Context.cpp
@@ -2000,6 +2000,27 @@
return mExtensionStrings.size();
}
+void Context::beginTransformFeedback(GLenum primitiveMode)
+{
+ TransformFeedback *transformFeedback = getState().getCurrentTransformFeedback();
+ ASSERT(transformFeedback != nullptr);
+ ASSERT(!transformFeedback->isPaused());
+
+ transformFeedback->begin(primitiveMode, getState().getProgram());
+}
+
+bool Context::hasActiveTransformFeedback(GLuint program) const
+{
+ for (auto pair : mTransformFeedbackMap)
+ {
+ if (pair.second != nullptr && pair.second->hasBoundProgram(program))
+ {
+ return true;
+ }
+ }
+ return false;
+}
+
void Context::initCaps(GLuint clientVersion)
{
mCaps = mRenderer->getRendererCaps();
diff --git a/src/libANGLE/Context.h b/src/libANGLE/Context.h
index 0a95b3f..af82e9c 100644
--- a/src/libANGLE/Context.h
+++ b/src/libANGLE/Context.h
@@ -370,6 +370,10 @@
Error flush();
Error finish();
+ void beginTransformFeedback(GLenum primitiveMode);
+
+ bool hasActiveTransformFeedback(GLuint program) const;
+
void insertEventMarker(GLsizei length, const char *marker);
void pushGroupMarker(GLsizei length, const char *marker);
void popGroupMarker();
diff --git a/src/libANGLE/TransformFeedback.cpp b/src/libANGLE/TransformFeedback.cpp
index 6750351..6ee1700 100644
--- a/src/libANGLE/TransformFeedback.cpp
+++ b/src/libANGLE/TransformFeedback.cpp
@@ -8,6 +8,7 @@
#include "libANGLE/Buffer.h"
#include "libANGLE/Caps.h"
+#include "libANGLE/Program.h"
#include "libANGLE/renderer/ImplFactory.h"
#include "libANGLE/renderer/TransformFeedbackImpl.h"
@@ -21,6 +22,7 @@
mActive(false),
mPrimitiveMode(GL_NONE),
mPaused(false),
+ mProgram(nullptr),
mGenericBuffer(),
mIndexedBuffers(caps.maxTransformFeedbackSeparateAttributes)
{
@@ -29,6 +31,11 @@
TransformFeedback::~TransformFeedback()
{
+ if (mProgram)
+ {
+ mProgram->release();
+ mProgram = nullptr;
+ }
mGenericBuffer.set(nullptr);
for (size_t i = 0; i < mIndexedBuffers.size(); i++)
{
@@ -48,12 +55,13 @@
return mLabel;
}
-void TransformFeedback::begin(GLenum primitiveMode)
+void TransformFeedback::begin(GLenum primitiveMode, Program *program)
{
mActive = true;
mPrimitiveMode = primitiveMode;
mPaused = false;
mImplementation->begin(primitiveMode);
+ bindProgram(program);
}
void TransformFeedback::end()
@@ -62,6 +70,11 @@
mPrimitiveMode = GL_NONE;
mPaused = false;
mImplementation->end();
+ if (mProgram)
+ {
+ mProgram->release();
+ mProgram = nullptr;
+ }
}
void TransformFeedback::pause()
@@ -91,6 +104,27 @@
return mPrimitiveMode;
}
+void TransformFeedback::bindProgram(Program *program)
+{
+ if (mProgram != program)
+ {
+ if (mProgram != nullptr)
+ {
+ mProgram->release();
+ }
+ mProgram = program;
+ if (mProgram != nullptr)
+ {
+ mProgram->addRef();
+ }
+ }
+}
+
+bool TransformFeedback::hasBoundProgram(GLuint program) const
+{
+ return mProgram != nullptr && mProgram->id() == program;
+}
+
void TransformFeedback::bindGenericBuffer(Buffer *buffer)
{
mGenericBuffer.set(buffer);
diff --git a/src/libANGLE/TransformFeedback.h b/src/libANGLE/TransformFeedback.h
index b2d7236..9e8af2e 100644
--- a/src/libANGLE/TransformFeedback.h
+++ b/src/libANGLE/TransformFeedback.h
@@ -24,6 +24,7 @@
{
class Buffer;
struct Caps;
+class Program;
class TransformFeedback final : public RefCountObject, public LabeledObject
{
@@ -34,7 +35,7 @@
void setLabel(const std::string &label) override;
const std::string &getLabel() const override;
- void begin(GLenum primitiveMode);
+ void begin(GLenum primitiveMode, Program *program);
void end();
void pause();
void resume();
@@ -43,6 +44,8 @@
bool isPaused() const;
GLenum getPrimitiveMode() const;
+ bool hasBoundProgram(GLuint program) const;
+
void bindGenericBuffer(Buffer *buffer);
const BindingPointer<Buffer> &getGenericBuffer() const;
@@ -56,6 +59,8 @@
const rx::TransformFeedbackImpl *getImplementation() const;
private:
+ void bindProgram(Program *program);
+
rx::TransformFeedbackImpl* mImplementation;
std::string mLabel;
@@ -64,6 +69,8 @@
GLenum mPrimitiveMode;
bool mPaused;
+ Program *mProgram;
+
BindingPointer<Buffer> mGenericBuffer;
std::vector<OffsetBindingPointer<Buffer>> mIndexedBuffers;
};
diff --git a/src/libANGLE/TransformFeedback_unittest.cpp b/src/libANGLE/TransformFeedback_unittest.cpp
index c304970..56d02e2 100644
--- a/src/libANGLE/TransformFeedback_unittest.cpp
+++ b/src/libANGLE/TransformFeedback_unittest.cpp
@@ -71,7 +71,7 @@
EXPECT_FALSE(mFeedback->isActive());
EXPECT_CALL(*mImpl, begin(GL_TRIANGLES));
- mFeedback->begin(GL_TRIANGLES);
+ mFeedback->begin(GL_TRIANGLES, nullptr);
EXPECT_TRUE(mFeedback->isActive());
EXPECT_EQ(static_cast<GLenum>(GL_TRIANGLES), mFeedback->getPrimitiveMode());
EXPECT_CALL(*mImpl, end());
@@ -85,7 +85,7 @@
EXPECT_FALSE(mFeedback->isActive());
EXPECT_CALL(*mImpl, begin(GL_TRIANGLES));
- mFeedback->begin(GL_TRIANGLES);
+ mFeedback->begin(GL_TRIANGLES, nullptr);
EXPECT_FALSE(mFeedback->isPaused());
EXPECT_CALL(*mImpl, pause());
mFeedback->pause();
diff --git a/src/libANGLE/validationES.cpp b/src/libANGLE/validationES.cpp
index 09471a7..a994a9d 100644
--- a/src/libANGLE/validationES.cpp
+++ b/src/libANGLE/validationES.cpp
@@ -2423,6 +2423,19 @@
return true;
}
+bool ValidateLinkProgram(Context *context, GLuint program)
+{
+ if (context->hasActiveTransformFeedback(program))
+ {
+ // ES 3.0.4 section 2.15 page 91
+ context->recordError(Error(GL_INVALID_OPERATION,
+ "Cannot link program while program is associated with an active "
+ "transform feedback object."));
+ return false;
+ }
+ return true;
+}
+
bool ValidateProgramBinaryBase(Context *context,
GLuint program,
GLenum binaryFormat,
@@ -2443,6 +2456,15 @@
return false;
}
+ if (context->hasActiveTransformFeedback(program))
+ {
+ // ES 3.0.4 section 2.15 page 91
+ context->recordError(Error(GL_INVALID_OPERATION,
+ "Cannot change program binary while program is associated with "
+ "an active transform feedback object."));
+ return false;
+ }
+
return true;
}
@@ -2468,6 +2490,45 @@
return true;
}
+bool ValidateUseProgram(Context *context, GLuint program)
+{
+ if (program != 0)
+ {
+ Program *programObject = context->getProgram(program);
+ if (!programObject)
+ {
+ // ES 3.1.0 section 7.3 page 72
+ if (context->getShader(program))
+ {
+ context->recordError(
+ Error(GL_INVALID_OPERATION,
+ "Attempted to use a single shader instead of a shader program."));
+ return false;
+ }
+ else
+ {
+ context->recordError(Error(GL_INVALID_VALUE, "Program invalid."));
+ return false;
+ }
+ }
+ if (!programObject->isLinked())
+ {
+ context->recordError(Error(GL_INVALID_OPERATION, "Program not linked."));
+ return false;
+ }
+ }
+ if (context->getState().isTransformFeedbackActiveUnpaused())
+ {
+ // ES 3.0.4 section 2.15 page 91
+ context->recordError(
+ Error(GL_INVALID_OPERATION,
+ "Cannot change active program while transform feedback is unpaused."));
+ return false;
+ }
+
+ return true;
+}
+
bool ValidateCopyTexImage2D(ValidationContext *context,
GLenum target,
GLint level,
diff --git a/src/libANGLE/validationES.h b/src/libANGLE/validationES.h
index ff39640..3e4a52d 100644
--- a/src/libANGLE/validationES.h
+++ b/src/libANGLE/validationES.h
@@ -201,6 +201,7 @@
bool ValidateBindVertexArrayBase(Context *context, GLuint array);
+bool ValidateLinkProgram(Context *context, GLuint program);
bool ValidateProgramBinaryBase(Context *context,
GLuint program,
GLenum binaryFormat,
@@ -212,6 +213,7 @@
GLsizei *length,
GLenum *binaryFormat,
void *binary);
+bool ValidateUseProgram(Context *context, GLuint program);
bool ValidateCopyTexImage2D(ValidationContext *context,
GLenum target,
diff --git a/src/libANGLE/validationES3.cpp b/src/libANGLE/validationES3.cpp
index d2db811..24ab91a 100644
--- a/src/libANGLE/validationES3.cpp
+++ b/src/libANGLE/validationES3.cpp
@@ -1940,4 +1940,34 @@
return true;
}
+bool ValidateBeginTransformFeedback(Context *context, GLenum primitiveMode)
+{
+ if (context->getClientVersion() < 3)
+ {
+ context->recordError(Error(GL_INVALID_OPERATION, "Context does not support GLES3."));
+ return false;
+ }
+ switch (primitiveMode)
+ {
+ case GL_TRIANGLES:
+ case GL_LINES:
+ case GL_POINTS:
+ break;
+
+ default:
+ context->recordError(Error(GL_INVALID_ENUM, "Invalid primitive mode."));
+ return false;
+ }
+
+ TransformFeedback *transformFeedback = context->getState().getCurrentTransformFeedback();
+ ASSERT(transformFeedback != nullptr);
+
+ if (transformFeedback->isActive())
+ {
+ context->recordError(Error(GL_INVALID_OPERATION, "Transform feedback is already active."));
+ return false;
+ }
+ return true;
+}
+
} // namespace gl
diff --git a/src/libANGLE/validationES3.h b/src/libANGLE/validationES3.h
index 9884575..0db78cf 100644
--- a/src/libANGLE/validationES3.h
+++ b/src/libANGLE/validationES3.h
@@ -289,6 +289,8 @@
bool ValidateGenOrDeleteES3(Context *context, GLint n);
bool ValidateGenOrDeleteCountES3(Context *context, GLint count);
+bool ValidateBeginTransformFeedback(Context *context, GLenum primitiveMode);
+
} // namespace gl
#endif // LIBANGLE_VALIDATION_ES3_H_
diff --git a/src/libGLESv2/entry_points_gles_2_0.cpp b/src/libGLESv2/entry_points_gles_2_0.cpp
index de6ba42..d1f46ee 100644
--- a/src/libGLESv2/entry_points_gles_2_0.cpp
+++ b/src/libGLESv2/entry_points_gles_2_0.cpp
@@ -2957,6 +2957,11 @@
Context *context = GetValidGlobalContext();
if (context)
{
+ if (!context->skipValidation() && !ValidateLinkProgram(context, program))
+ {
+ return;
+ }
+
Program *programObject = GetValidProgram(context, program);
if (!programObject)
{
@@ -3822,25 +3827,8 @@
Context *context = GetValidGlobalContext();
if (context)
{
- Program *programObject = context->getProgram(program);
-
- if (!programObject && program != 0)
+ if (!context->skipValidation() && !ValidateUseProgram(context, program))
{
- if (context->getShader(program))
- {
- context->recordError(Error(GL_INVALID_OPERATION));
- return;
- }
- else
- {
- context->recordError(Error(GL_INVALID_VALUE));
- return;
- }
- }
-
- if (program != 0 && !programObject->isLinked())
- {
- context->recordError(Error(GL_INVALID_OPERATION));
return;
}
diff --git a/src/libGLESv2/entry_points_gles_3_0.cpp b/src/libGLESv2/entry_points_gles_3_0.cpp
index dda9bd1..5687224 100644
--- a/src/libGLESv2/entry_points_gles_3_0.cpp
+++ b/src/libGLESv2/entry_points_gles_3_0.cpp
@@ -746,41 +746,12 @@
Context *context = GetValidGlobalContext();
if (context)
{
- if (context->getClientVersion() < 3)
+ if (!context->skipValidation() && !ValidateBeginTransformFeedback(context, primitiveMode))
{
- context->recordError(Error(GL_INVALID_OPERATION));
return;
}
- switch (primitiveMode)
- {
- case GL_TRIANGLES:
- case GL_LINES:
- case GL_POINTS:
- break;
-
- default:
- context->recordError(Error(GL_INVALID_ENUM));
- return;
- }
-
- TransformFeedback *transformFeedback = context->getState().getCurrentTransformFeedback();
- ASSERT(transformFeedback != NULL);
-
- if (transformFeedback->isActive())
- {
- context->recordError(Error(GL_INVALID_OPERATION));
- return;
- }
-
- if (transformFeedback->isPaused())
- {
- transformFeedback->resume();
- }
- else
- {
- transformFeedback->begin(primitiveMode);
- }
+ context->beginTransformFeedback(primitiveMode);
}
}
diff --git a/src/tests/deqp_support/deqp_gles3_test_expectations.txt b/src/tests/deqp_support/deqp_gles3_test_expectations.txt
index 1491fd1..0ab2249 100644
--- a/src/tests/deqp_support/deqp_gles3_test_expectations.txt
+++ b/src/tests/deqp_support/deqp_gles3_test_expectations.txt
@@ -54,8 +54,6 @@
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage2d_invalid_size = FAIL
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d = FAIL
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.texture.compressedtexsubimage3d_invalid_buffer_target = FAIL
-1101 WIN LINUX : dEQP-GLES3.functional.negative_api.shader.link_program = FAIL
-1101 WIN LINUX : dEQP-GLES3.functional.negative_api.shader.use_program = FAIL
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.shader.sampler_parameterfv = FAIL
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.fragment.begin_query = FAIL
1101 WIN LINUX : dEQP-GLES3.functional.negative_api.vertex_array.vertex_attrib_i_pointer = FAIL
diff --git a/src/tests/gl_tests/ProvokingVertexTest.cpp b/src/tests/gl_tests/ProvokingVertexTest.cpp
index a407616..adf0b1a 100644
--- a/src/tests/gl_tests/ProvokingVertexTest.cpp
+++ b/src/tests/gl_tests/ProvokingVertexTest.cpp
@@ -161,9 +161,11 @@
GLint vertexData[] = {1, 2, 3, 1, 2, 3};
glVertexAttribIPointer(mIntAttribLocation, 1, GL_INT, 0, vertexData);
+ glUseProgram(mProgram);
glBeginTransformFeedback(GL_TRIANGLES);
drawQuad(mProgram, "position", 0.5f);
glEndTransformFeedback();
+ glUseProgram(0);
GLint pixelValue = 0;
glReadPixels(0, 0, 1, 1, GL_RED_INTEGER, GL_INT, &pixelValue);
diff --git a/src/tests/gl_tests/TransformFeedbackTest.cpp b/src/tests/gl_tests/TransformFeedbackTest.cpp
index 48ca9d4..b9a000b 100644
--- a/src/tests/gl_tests/TransformFeedbackTest.cpp
+++ b/src/tests/gl_tests/TransformFeedbackTest.cpp
@@ -132,6 +132,8 @@
glEndQuery(GL_TRANSFORM_FEEDBACK_PRIMITIVES_WRITTEN);
glEndTransformFeedback();
+ glUseProgram(0);
+
// Check how many primitives were written and verify that some were written even if
// no pixels were rendered
GLuint primitivesWritten = 0;
@@ -305,6 +307,8 @@
tfVaryings, GL_INTERLEAVED_ATTRIBS);
ASSERT_NE(0u, mProgram);
+ glUseProgram(mProgram);
+
glBindTransformFeedback(GL_TRANSFORM_FEEDBACK, mTransformFeedback);
glBindBufferBase(GL_TRANSFORM_FEEDBACK_BUFFER, 0, mTransformFeedbackBuffer);
@@ -325,6 +329,8 @@
glEndTransformFeedback();
ASSERT_GL_NO_ERROR();
+ glUseProgram(0);
+
GLvoid *mappedBuffer =
glMapBufferRange(GL_TRANSFORM_FEEDBACK_BUFFER, 0, sizeof(float) * 6, GL_MAP_READ_BIT);
ASSERT_NE(nullptr, mappedBuffer);
@@ -678,9 +684,11 @@
glVertexAttribPointer(attrib1Loc, 2, GL_FLOAT, GL_FALSE, 0, attrib1Data);
glVertexAttribPointer(attrib2Loc, 2, GL_FLOAT, GL_FALSE, 0, attrib2Data);
+ glUseProgram(mProgram);
glBeginTransformFeedback(GL_TRIANGLES);
drawQuad(mProgram, "position", 0.5f);
glEndTransformFeedback();
+ glUseProgram(0);
ASSERT_GL_NO_ERROR();
const GLvoid *mapPointer =
diff --git a/src/tests/test_utils/ANGLETest.cpp b/src/tests/test_utils/ANGLETest.cpp
index b308e11..83d9380 100644
--- a/src/tests/test_utils/ANGLETest.cpp
+++ b/src/tests/test_utils/ANGLETest.cpp
@@ -166,9 +166,14 @@
GLfloat positionAttribZ,
GLfloat positionAttribXYScale)
{
- GLint positionLocation = glGetAttribLocation(program, positionAttribName.c_str());
+ GLint previousProgram = 0;
+ glGetIntegerv(GL_CURRENT_PROGRAM, &previousProgram);
+ if (previousProgram != static_cast<GLint>(program))
+ {
+ glUseProgram(program);
+ }
- glUseProgram(program);
+ GLint positionLocation = glGetAttribLocation(program, positionAttribName.c_str());
const GLfloat vertices[] = {
-1.0f * positionAttribXYScale, 1.0f * positionAttribXYScale, positionAttribZ,
@@ -188,7 +193,10 @@
glDisableVertexAttribArray(positionLocation);
glVertexAttribPointer(positionLocation, 4, GL_FLOAT, GL_FALSE, 0, NULL);
- glUseProgram(0);
+ if (previousProgram != static_cast<GLint>(program))
+ {
+ glUseProgram(previousProgram);
+ }
}
void ANGLETest::drawIndexedQuad(GLuint program,