Commit c3e55a43 by Olli Etuaho Committed by Commit Bot

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/332141Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent 6596c465
......@@ -2000,6 +2000,27 @@ size_t Context::getExtensionStringCount() const
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();
......
......@@ -370,6 +370,10 @@ class Context final : public ValidationContext
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();
......
......@@ -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 @@ TransformFeedback::TransformFeedback(rx::ImplFactory *implFactory, GLuint id, co
mActive(false),
mPrimitiveMode(GL_NONE),
mPaused(false),
mProgram(nullptr),
mGenericBuffer(),
mIndexedBuffers(caps.maxTransformFeedbackSeparateAttributes)
{
......@@ -29,6 +31,11 @@ TransformFeedback::TransformFeedback(rx::ImplFactory *implFactory, GLuint id, co
TransformFeedback::~TransformFeedback()
{
if (mProgram)
{
mProgram->release();
mProgram = nullptr;
}
mGenericBuffer.set(nullptr);
for (size_t i = 0; i < mIndexedBuffers.size(); i++)
{
......@@ -48,12 +55,13 @@ const std::string &TransformFeedback::getLabel() const
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 @@ void TransformFeedback::end()
mPrimitiveMode = GL_NONE;
mPaused = false;
mImplementation->end();
if (mProgram)
{
mProgram->release();
mProgram = nullptr;
}
}
void TransformFeedback::pause()
......@@ -91,6 +104,27 @@ GLenum TransformFeedback::getPrimitiveMode() const
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);
......
......@@ -24,6 +24,7 @@ namespace gl
{
class Buffer;
struct Caps;
class Program;
class TransformFeedback final : public RefCountObject, public LabeledObject
{
......@@ -34,7 +35,7 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
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 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
bool isPaused() const;
GLenum getPrimitiveMode() const;
bool hasBoundProgram(GLuint program) const;
void bindGenericBuffer(Buffer *buffer);
const BindingPointer<Buffer> &getGenericBuffer() const;
......@@ -56,6 +59,8 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
const rx::TransformFeedbackImpl *getImplementation() const;
private:
void bindProgram(Program *program);
rx::TransformFeedbackImpl* mImplementation;
std::string mLabel;
......@@ -64,6 +69,8 @@ class TransformFeedback final : public RefCountObject, public LabeledObject
GLenum mPrimitiveMode;
bool mPaused;
Program *mProgram;
BindingPointer<Buffer> mGenericBuffer;
std::vector<OffsetBindingPointer<Buffer>> mIndexedBuffers;
};
......
......@@ -71,7 +71,7 @@ TEST_F(TransformFeedbackTest, SideEffectsOfStartAndStop)
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 @@ TEST_F(TransformFeedbackTest, SideEffectsOfPauseAndResume)
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();
......
......@@ -2423,6 +2423,19 @@ bool ValidateBindVertexArrayBase(Context *context, GLuint array)
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 @@ bool ValidateProgramBinaryBase(Context *context,
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 @@ bool ValidateGetProgramBinaryBase(Context *context,
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,
......
......@@ -201,6 +201,7 @@ bool ValidateEGLImageTargetRenderbufferStorageOES(Context *context,
bool ValidateBindVertexArrayBase(Context *context, GLuint array);
bool ValidateLinkProgram(Context *context, GLuint program);
bool ValidateProgramBinaryBase(Context *context,
GLuint program,
GLenum binaryFormat,
......@@ -212,6 +213,7 @@ bool ValidateGetProgramBinaryBase(Context *context,
GLsizei *length,
GLenum *binaryFormat,
void *binary);
bool ValidateUseProgram(Context *context, GLuint program);
bool ValidateCopyTexImage2D(ValidationContext *context,
GLenum target,
......
......@@ -1940,4 +1940,34 @@ bool ValidateGenOrDeleteCountES3(Context *context, GLint count)
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
......@@ -289,6 +289,8 @@ bool ValidateDeleteVertexArrays(Context *context, GLint n, const GLuint *arrays)
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_
......@@ -2957,6 +2957,11 @@ void GL_APIENTRY LinkProgram(GLuint program)
Context *context = GetValidGlobalContext();
if (context)
{
if (!context->skipValidation() && !ValidateLinkProgram(context, program))
{
return;
}
Program *programObject = GetValidProgram(context, program);
if (!programObject)
{
......@@ -3822,25 +3827,8 @@ void GL_APIENTRY UseProgram(GLuint program)
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;
}
......
......@@ -746,41 +746,12 @@ void GL_APIENTRY BeginTransformFeedback(GLenum primitiveMode)
Context *context = GetValidGlobalContext();
if (context)
{
if (context->getClientVersion() < 3)
{
context->recordError(Error(GL_INVALID_OPERATION));
return;
}
switch (primitiveMode)
if (!context->skipValidation() && !ValidateBeginTransformFeedback(context, 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);
}
}
......
......@@ -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
......
......@@ -161,9 +161,11 @@ TEST_P(ProvokingVertexTest, FlatTriWithTransformFeedback)
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);
......
......@@ -132,6 +132,8 @@ TEST_P(TransformFeedbackTest, ZeroSizedViewport)
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 @@ TEST_P(TransformFeedbackTest, VertexOnly)
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 @@ TEST_P(TransformFeedbackTest, VertexOnly)
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 @@ TEST_P(TransformFeedbackTest, PackingBug)
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 =
......
......@@ -166,9 +166,14 @@ void ANGLETest::drawQuad(GLuint program,
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 @@ void ANGLETest::drawQuad(GLuint program,
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,
......
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