Move more draw call validation to the API.
The GL expects us to reject invalid draw calls before we start
doing any work, so we can prevent internal unnecessary state
changes.
Also update the program binary's cached sampler data when we
validate. The previous patch was breaking draw calls in Google
Earth WebGL.
BUG=angle:571
BUG=390412
Change-Id: I1c4e204ae2467afc36b76af975a3a49e26349639
Reviewed-on: https://chromium-review.googlesource.com/206482
Tested-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libGLESv2/Context.cpp b/src/libGLESv2/Context.cpp
index c829a72..5712c52 100644
--- a/src/libGLESv2/Context.cpp
+++ b/src/libGLESv2/Context.cpp
@@ -1336,6 +1336,11 @@
}
+GLuint Context::getCurrentProgram() const
+{
+ return mState.currentProgram;
+}
+
void Context::bindTransformFeedback(GLuint transformFeedback)
{
TransformFeedback *transformFeedbackObject = getTransformFeedback(transformFeedback);
@@ -1479,7 +1484,7 @@
return getCurrentVertexArray()->getElementArrayBuffer();
}
-ProgramBinary *Context::getCurrentProgramBinary()
+ProgramBinary *Context::getCurrentProgramBinary() const
{
return mCurrentProgramBinary.get();
}
@@ -2829,13 +2834,10 @@
void Context::drawArrays(GLenum mode, GLint first, GLsizei count, GLsizei instances)
{
- if (!mState.currentProgram)
- {
- return gl::error(GL_INVALID_OPERATION);
- }
+ ASSERT(mState.currentProgram);
ProgramBinary *programBinary = getCurrentProgramBinary();
- programBinary->applyUniforms();
+ programBinary->updateSamplerMapping();
Texture *vsTextures[IMPLEMENTATION_MAX_VERTEX_TEXTURE_IMAGE_UNITS];
TextureType vsTextureTypes[IMPLEMENTATION_MAX_VERTEX_TEXTURE_IMAGE_UNITS];
@@ -2883,11 +2885,6 @@
return;
}
- if (!programBinary->validateSamplers(NULL))
- {
- return gl::error(GL_INVALID_OPERATION);
- }
-
if (!skipDraw(mode))
{
mRenderer->drawArrays(mode, count, instances, transformFeedbackActive);
@@ -2901,19 +2898,10 @@
void Context::drawElements(GLenum mode, GLsizei count, GLenum type, const GLvoid *indices, GLsizei instances)
{
- if (!mState.currentProgram)
- {
- return gl::error(GL_INVALID_OPERATION);
- }
-
- VertexArray *vao = getCurrentVertexArray();
- if (!indices && !vao->getElementArrayBuffer())
- {
- return gl::error(GL_INVALID_OPERATION);
- }
+ ASSERT(mState.currentProgram);
ProgramBinary *programBinary = getCurrentProgramBinary();
- programBinary->applyUniforms();
+ programBinary->updateSamplerMapping();
Texture *vsTextures[IMPLEMENTATION_MAX_VERTEX_TEXTURE_IMAGE_UNITS];
TextureType vsTextureTypes[IMPLEMENTATION_MAX_VERTEX_TEXTURE_IMAGE_UNITS];
@@ -2940,6 +2928,7 @@
applyState(mode);
+ VertexArray *vao = getCurrentVertexArray();
rx::TranslatedIndexData indexInfo;
GLenum err = mRenderer->applyIndexBuffer(indices, vao->getElementArrayBuffer(), count, mode, type, &indexInfo);
if (err != GL_NO_ERROR)
@@ -2972,11 +2961,6 @@
return;
}
- if (!programBinary->validateSamplers(NULL))
- {
- return gl::error(GL_INVALID_OPERATION);
- }
-
if (!skipDraw(mode))
{
mRenderer->drawElements(mode, count, type, indices, vao->getElementArrayBuffer(), indexInfo, instances);
diff --git a/src/libGLESv2/Context.h b/src/libGLESv2/Context.h
index 9dc4d16..32b7c1d 100644
--- a/src/libGLESv2/Context.h
+++ b/src/libGLESv2/Context.h
@@ -306,6 +306,7 @@
void useProgram(GLuint program);
void linkProgram(GLuint program);
void setProgramBinary(GLuint program, const void *binary, GLint length);
+ GLuint getCurrentProgram() const;
void bindTransformFeedback(GLuint transformFeedback);
void beginQuery(GLenum target, GLuint query);
@@ -341,7 +342,7 @@
Buffer *getTargetBuffer(GLenum target) const;
Buffer *getArrayBuffer();
Buffer *getElementArrayBuffer() const;
- ProgramBinary *getCurrentProgramBinary();
+ ProgramBinary *getCurrentProgramBinary() const;
Texture *getTargetTexture(GLenum target) const;
Texture2D *getTexture2D() const;
diff --git a/src/libGLESv2/ProgramBinary.cpp b/src/libGLESv2/ProgramBinary.cpp
index 2efe8b7..5a22500 100644
--- a/src/libGLESv2/ProgramBinary.cpp
+++ b/src/libGLESv2/ProgramBinary.cpp
@@ -150,6 +150,7 @@
mUsedPixelSamplerRange(0),
mUsesPointSize(false),
mShaderVersion(100),
+ mDirtySamplerMapping(true),
mVertexUniformStorage(NULL),
mFragmentUniformStorage(NULL),
mValidated(false),
@@ -765,6 +766,13 @@
}
}
else UNREACHABLE();
+
+ // Set a special flag if we change a sampler uniform
+ if (IsSampler(targetUniform->type) &&
+ (memcmp(targetUniform->data, v, sizeof(GLint)) != 0))
+ {
+ mDirtySamplerMapping = true;
+ }
}
void ProgramBinary::setUniform2iv(GLint location, GLsizei count, const GLint *v)
@@ -909,9 +917,15 @@
}
}
-// Applies all the uniforms set for this program object to the renderer
-void ProgramBinary::applyUniforms()
+void ProgramBinary::updateSamplerMapping()
{
+ if (!mDirtySamplerMapping)
+ {
+ return;
+ }
+
+ mDirtySamplerMapping = false;
+
// Retrieve sampler uniform values
for (size_t uniformIndex = 0; uniformIndex < mUniforms.size(); uniformIndex++)
{
@@ -922,7 +936,7 @@
if (IsSampler(targetUniform->type))
{
int count = targetUniform->elementCount();
- GLint (*v)[4] = (GLint(*)[4])targetUniform->data;
+ GLint (*v)[4] = reinterpret_cast<GLint(*)[4]>(targetUniform->data);
if (targetUniform->isReferencedByFragmentShader())
{
@@ -958,6 +972,12 @@
}
}
}
+}
+
+// Applies all the uniforms set for this program object to the renderer
+void ProgramBinary::applyUniforms()
+{
+ updateSamplerMapping();
mRenderer->applyUniforms(*this);
@@ -2615,6 +2635,7 @@
// if any two active samplers in a program are of different types, but refer to the same
// texture image unit, and this is the current program, then ValidateProgram will fail, and
// DrawArrays and DrawElements will issue the INVALID_OPERATION error.
+ updateSamplerMapping();
const unsigned int maxCombinedTextureImageUnits = mRenderer->getMaxCombinedTextureImageUnits();
TextureType textureUnitType[IMPLEMENTATION_MAX_COMBINED_TEXTURE_IMAGE_UNITS];
@@ -2800,6 +2821,7 @@
mUsedPixelSamplerRange = 0;
mUsesPointSize = false;
mShaderVersion = 0;
+ mDirtySamplerMapping = true;
SafeDeleteContainer(mUniforms);
SafeDeleteContainer(mUniformBlocks);
diff --git a/src/libGLESv2/ProgramBinary.h b/src/libGLESv2/ProgramBinary.h
index aef4dad..c73ce66 100644
--- a/src/libGLESv2/ProgramBinary.h
+++ b/src/libGLESv2/ProgramBinary.h
@@ -162,6 +162,7 @@
void validate(InfoLog &infoLog);
bool validateSamplers(InfoLog *infoLog);
bool isValidated() const;
+ void updateSamplerMapping();
unsigned int getSerial() const;
int getShaderVersion() const;
@@ -290,6 +291,7 @@
GLuint mUsedPixelSamplerRange;
bool mUsesPointSize;
int mShaderVersion;
+ bool mDirtySamplerMapping;
std::vector<LinkedUniform*> mUniforms;
std::vector<UniformBlock*> mUniformBlocks;
diff --git a/src/libGLESv2/validationES.cpp b/src/libGLESv2/validationES.cpp
index 2324a4b..6ee4cae 100644
--- a/src/libGLESv2/validationES.cpp
+++ b/src/libGLESv2/validationES.cpp
@@ -19,6 +19,7 @@
#include "libGLESv2/Query.h"
#include "libGLESv2/ProgramBinary.h"
#include "libGLESv2/TransformFeedback.h"
+#include "libGLESv2/VertexArray.h"
#include "common/mathutil.h"
#include "common/utilities.h"
@@ -1346,6 +1347,17 @@
return gl::error(GL_INVALID_FRAMEBUFFER_OPERATION, false);
}
+ if (!context->getCurrentProgram())
+ {
+ return gl::error(GL_INVALID_OPERATION, false);
+ }
+
+ gl::ProgramBinary *programBinary = context->getCurrentProgramBinary();
+ if (!programBinary->validateSamplers(NULL))
+ {
+ return gl::error(GL_INVALID_OPERATION, false);
+ }
+
// No-op if zero count
return (count > 0);
}
@@ -1422,6 +1434,12 @@
return gl::error(GL_INVALID_OPERATION, false);
}
+ gl::VertexArray *vao = context->getCurrentVertexArray();
+ if (!indices && !vao->getElementArrayBuffer())
+ {
+ return gl::error(GL_INVALID_OPERATION, false);
+ }
+
if (!ValidateDrawBase(context, mode, count))
{
return false;
diff --git a/tests/angle_tests/TextureTest.cpp b/tests/angle_tests/TextureTest.cpp
index f5fff41..0fb046c 100644
--- a/tests/angle_tests/TextureTest.cpp
+++ b/tests/angle_tests/TextureTest.cpp
@@ -16,12 +16,17 @@
virtual void SetUp()
{
ANGLETest::SetUp();
- glGenTextures(1, &mTexture);
+ glGenTextures(1, &mTexture2D);
+ glGenTextures(1, &mTextureCube);
- glBindTexture(GL_TEXTURE_2D, mTexture);
+ glBindTexture(GL_TEXTURE_2D, mTexture2D);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 1, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, NULL);
EXPECT_GL_NO_ERROR();
+ glBindTexture(GL_TEXTURE_CUBE_MAP, mTextureCube);
+ glTexStorage2DEXT(GL_TEXTURE_CUBE_MAP, 1, GL_RGBA8, 1, 1);
+ EXPECT_GL_NO_ERROR();
+
ASSERT_GL_NO_ERROR();
const std::string vertexShaderSource = SHADER_SOURCE
@@ -37,7 +42,7 @@
}
);
- const std::string fragmentShaderSource = SHADER_SOURCE
+ const std::string fragmentShaderSource2D = SHADER_SOURCE
(
precision highp float;
uniform sampler2D tex;
@@ -49,32 +54,51 @@
}
);
- mProgram = compileProgram(vertexShaderSource, fragmentShaderSource);
- if (mProgram == 0)
+ const std::string fragmentShaderSourceCube = SHADER_SOURCE
+ (
+ precision highp float;
+ uniform sampler2D tex2D;
+ uniform samplerCube texCube;
+ varying vec2 texcoord;
+
+ void main()
+ {
+ gl_FragColor = texture2D(tex2D, texcoord);
+ gl_FragColor += textureCube(texCube, vec3(texcoord, 0));
+ }
+ );
+
+ m2DProgram = compileProgram(vertexShaderSource, fragmentShaderSource2D);
+ mCubeProgram = compileProgram(vertexShaderSource, fragmentShaderSourceCube);
+ if (m2DProgram == 0 || mCubeProgram == 0)
{
FAIL() << "shader compilation failed.";
}
- mTextureUniformLocation = glGetUniformLocation(mProgram, "tex");
+ mTexture2DUniformLocation = glGetUniformLocation(m2DProgram, "tex");
}
virtual void TearDown()
{
- glDeleteTextures(1, &mTexture);
- glDeleteProgram(mProgram);
+ glDeleteTextures(1, &mTexture2D);
+ glDeleteTextures(1, &mTextureCube);
+ glDeleteProgram(m2DProgram);
+ glDeleteProgram(mCubeProgram);
ANGLETest::TearDown();
}
- GLuint mTexture;
+ GLuint mTexture2D;
+ GLuint mTextureCube;
- GLuint mProgram;
- GLint mTextureUniformLocation;
+ GLuint m2DProgram;
+ GLuint mCubeProgram;
+ GLint mTexture2DUniformLocation;
};
TEST_F(TextureTest, negative_api_subimage)
{
- glBindTexture(GL_TEXTURE_2D, mTexture);
+ glBindTexture(GL_TEXTURE_2D, mTexture2D);
EXPECT_GL_ERROR(GL_NO_ERROR);
const GLubyte *pixels[20] = { 0 };
@@ -84,13 +108,13 @@
TEST_F(TextureTest, zero_sized_uploads)
{
- glBindTexture(GL_TEXTURE_2D, mTexture);
+ glBindTexture(GL_TEXTURE_2D, mTexture2D);
EXPECT_GL_ERROR(GL_NO_ERROR);
// Use the texture first to make sure it's in video memory
- glUseProgram(mProgram);
- glUniform1i(mTextureUniformLocation, 0);
- drawQuad(mProgram, "position", 0.5f);
+ glUseProgram(m2DProgram);
+ glUniform1i(mTexture2DUniformLocation, 0);
+ drawQuad(m2DProgram, "position", 0.5f);
const GLubyte *pixel[4] = { 0 };
@@ -103,3 +127,23 @@
glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, 1, 0, GL_RGBA, GL_UNSIGNED_BYTE, pixel);
EXPECT_GL_NO_ERROR();
}
+
+// Test drawing with two texture types, to trigger an ANGLE bug in validation
+TEST_F(TextureTest, cube_map_bug)
+{
+ glActiveTexture(GL_TEXTURE0);
+ glBindTexture(GL_TEXTURE_2D, mTexture2D);
+ glActiveTexture(GL_TEXTURE1);
+ glBindTexture(GL_TEXTURE_CUBE_MAP, mTextureCube);
+ EXPECT_GL_ERROR(GL_NO_ERROR);
+
+ glUseProgram(mCubeProgram);
+ GLint tex2DUniformLocation = glGetUniformLocation(mCubeProgram, "tex2D");
+ GLint texCubeUniformLocation = glGetUniformLocation(mCubeProgram, "texCube");
+ EXPECT_NE(-1, tex2DUniformLocation);
+ EXPECT_NE(-1, texCubeUniformLocation);
+ glUniform1i(tex2DUniformLocation, 0);
+ glUniform1i(texCubeUniformLocation, 1);
+ drawQuad(mCubeProgram, "position", 0.5f);
+ EXPECT_GL_NO_ERROR();
+}
