Commit 0328b575 by Xinghua Cao Committed by Commit Bot

Bind all elements of unbound image arrays to unit zero

Spec GLSL ES 3.10, section 4.4.5, Any uniform sampler, image or atomic counter variable declared without a binding qualifier is initially bound to unit zero. If the binding qualifier is used with an array, the first element of the array takes the specified unit and each subsequent element takes the next consecutive unit. BUG=angleproject:1987 TEST=angle_end2end_tests:ComputeShaderTest Change-Id: I6a8188449a91bf3e8ded37e067205dcae4e47fa7 Reviewed-on: https://chromium-review.googlesource.com/547977 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org>
parent 31ecbd70
...@@ -310,12 +310,16 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context, ...@@ -310,12 +310,16 @@ LinkResult MemoryProgramCache::Deserialize(const Context *context,
unsigned int imageRangeLow = stream.readInt<unsigned int>(); unsigned int imageRangeLow = stream.readInt<unsigned int>();
unsigned int imageRangeHigh = stream.readInt<unsigned int>(); unsigned int imageRangeHigh = stream.readInt<unsigned int>();
state->mImageUniformRange = RangeUI(imageRangeLow, imageRangeHigh); state->mImageUniformRange = RangeUI(imageRangeLow, imageRangeHigh);
unsigned int imageCount = stream.readInt<unsigned int>(); unsigned int imageBindingCount = stream.readInt<unsigned int>();
for (unsigned int imageIndex = 0; imageIndex < imageCount; ++imageIndex) for (unsigned int imageIndex = 0; imageIndex < imageBindingCount; ++imageIndex)
{ {
GLuint boundImageUnit = stream.readInt<unsigned int>(); unsigned int elementCount = stream.readInt<unsigned int>();
size_t elementCount = stream.readInt<size_t>(); ImageBinding imageBinding(elementCount);
state->mImageBindings.emplace_back(ImageBinding(boundImageUnit, elementCount)); for (unsigned int i = 0; i < elementCount; ++i)
{
imageBinding.boundImageUnits[i] = stream.readInt<unsigned int>();
}
state->mImageBindings.emplace_back(imageBinding);
} }
unsigned int atomicCounterRangeLow = stream.readInt<unsigned int>(); unsigned int atomicCounterRangeLow = stream.readInt<unsigned int>();
...@@ -466,8 +470,11 @@ void MemoryProgramCache::Serialize(const Context *context, ...@@ -466,8 +470,11 @@ void MemoryProgramCache::Serialize(const Context *context,
stream.writeInt(state.getImageBindings().size()); stream.writeInt(state.getImageBindings().size());
for (const auto &imageBinding : state.getImageBindings()) for (const auto &imageBinding : state.getImageBindings())
{ {
stream.writeInt(imageBinding.boundImageUnit); stream.writeInt(imageBinding.boundImageUnits.size());
stream.writeInt(imageBinding.elementCount); for (size_t i = 0; i < imageBinding.boundImageUnits.size(); ++i)
{
stream.writeInt(imageBinding.boundImageUnits[i]);
}
} }
stream.writeInt(state.getAtomicCounterUniformRange().low()); stream.writeInt(state.getAtomicCounterUniformRange().low());
......
...@@ -1808,17 +1808,20 @@ void Program::linkSamplerAndImageBindings() ...@@ -1808,17 +1808,20 @@ void Program::linkSamplerAndImageBindings()
// If uniform is a image type, insert it into the mImageBindings array. // If uniform is a image type, insert it into the mImageBindings array.
for (unsigned int imageIndex : mState.mImageUniformRange) for (unsigned int imageIndex : mState.mImageUniformRange)
{ {
// ES3.1 (section 7.6.1) and GLSL ES3.1 (section 4.4.5), Uniform*i{v} // ES3.1 (section 7.6.1) and GLSL ES3.1 (section 4.4.5), Uniform*i{v} commands
// commands cannot load values into a uniform defined as an image, // cannot load values into a uniform defined as an image. if declare without a
// if declare without a binding qualifier, the image variable is // binding qualifier, any uniform image variable (include all elements of
// initially bound to unit zero. // unbound image array) shoud be bound to unit zero.
auto &imageUniform = mState.mUniforms[imageIndex]; auto &imageUniform = mState.mUniforms[imageIndex];
if (imageUniform.binding == -1) if (imageUniform.binding == -1)
{ {
imageUniform.binding = 0; mState.mImageBindings.emplace_back(ImageBinding(imageUniform.elementCount()));
}
else
{
mState.mImageBindings.emplace_back(
ImageBinding(imageUniform.binding, imageUniform.elementCount()));
} }
mState.mImageBindings.emplace_back(
ImageBinding(imageUniform.binding, imageUniform.elementCount()));
} }
high = low; high = low;
......
...@@ -199,10 +199,16 @@ struct TransformFeedbackVarying : public sh::Varying ...@@ -199,10 +199,16 @@ struct TransformFeedbackVarying : public sh::Varying
struct ImageBinding struct ImageBinding
{ {
ImageBinding(GLuint imageUnit, size_t count) : boundImageUnit(imageUnit), elementCount(count) {} ImageBinding(size_t count) : boundImageUnits(count, 0) {}
ImageBinding(GLuint imageUnit, size_t count)
{
for (size_t index = 0; index < count; ++index)
{
boundImageUnits.push_back(imageUnit + static_cast<GLuint>(index));
}
}
GLuint boundImageUnit; std::vector<GLuint> boundImageUnits;
size_t elementCount;
}; };
class ProgramState final : angle::NonCopyable class ProgramState final : angle::NonCopyable
......
...@@ -883,23 +883,20 @@ void StateManagerGL::setGenericShaderState(const gl::Context *context) ...@@ -883,23 +883,20 @@ void StateManagerGL::setGenericShaderState(const gl::Context *context)
ASSERT(context->getClientVersion() >= gl::ES_3_1 || program->getImageBindings().size() == 0); ASSERT(context->getClientVersion() >= gl::ES_3_1 || program->getImageBindings().size() == 0);
for (const gl::ImageBinding &imageUniform : program->getImageBindings()) for (const gl::ImageBinding &imageUniform : program->getImageBindings())
{ {
for (size_t imageUnitIndex = 0; imageUnitIndex < imageUniform.elementCount; for (GLuint imageUnitIndex : imageUniform.boundImageUnits)
imageUnitIndex++)
{ {
const gl::ImageUnit &imageUnit = glState.getImageUnit( const gl::ImageUnit &imageUnit = glState.getImageUnit(imageUnitIndex);
imageUniform.boundImageUnit + static_cast<GLuint>(imageUnitIndex));
const TextureGL *textureGL = SafeGetImplAs<TextureGL>(imageUnit.texture.get()); const TextureGL *textureGL = SafeGetImplAs<TextureGL>(imageUnit.texture.get());
if (textureGL) if (textureGL)
{ {
bindImageTexture(imageUniform.boundImageUnit + static_cast<GLuint>(imageUnitIndex), bindImageTexture(imageUnitIndex, textureGL->getTextureID(), imageUnit.level,
textureGL->getTextureID(), imageUnit.level, imageUnit.layered, imageUnit.layered, imageUnit.layer, imageUnit.access,
imageUnit.layer, imageUnit.access, imageUnit.format); imageUnit.format);
} }
else else
{ {
bindImageTexture(imageUniform.boundImageUnit + static_cast<GLuint>(imageUnitIndex), bindImageTexture(imageUnitIndex, 0, imageUnit.level, imageUnit.layered,
0, imageUnit.level, imageUnit.layered, imageUnit.layer, imageUnit.layer, imageUnit.access, imageUnit.format);
imageUnit.access, imageUnit.format);
} }
} }
} }
......
...@@ -246,11 +246,7 @@ TEST_P(ComputeShaderTest, DispatchCompute) ...@@ -246,11 +246,7 @@ TEST_P(ComputeShaderTest, DispatchCompute)
// Use image uniform to write texture in compute shader, and verify the content is expected. // Use image uniform to write texture in compute shader, and verify the content is expected.
TEST_P(ComputeShaderTest, BindImageTexture) TEST_P(ComputeShaderTest, BindImageTexture)
{ {
if (IsD3D11()) ANGLE_SKIP_TEST_IF(IsD3D11());
{
std::cout << "Test skipped on D3D11 because it is not implemented yet." << std::endl;
return;
}
GLTexture mTexture[2]; GLTexture mTexture[2];
GLFramebuffer mFramebuffer; GLFramebuffer mFramebuffer;
...@@ -316,6 +312,58 @@ TEST_P(ComputeShaderTest, BindImageTexture) ...@@ -316,6 +312,58 @@ TEST_P(ComputeShaderTest, BindImageTexture)
} }
} }
// When declare a image array without a binding qualifier, all elements are bound to unit zero.
TEST_P(ComputeShaderTest, ImageArrayWithoutBindingQualifier)
{
ANGLE_SKIP_TEST_IF(IsD3D11());
// TODO(xinghua.cao@intel.com): On AMD desktop OpenGL, bind two image variables to unit 0,
// only one variable is valid.
ANGLE_SKIP_TEST_IF(IsAMD() && IsDesktopOpenGL() && IsWindows());
GLTexture mTexture;
GLFramebuffer mFramebuffer;
const std::string csSource =
"#version 310 es\n"
"layout(local_size_x=2, local_size_y=2, local_size_z=1) in;\n"
"layout(r32ui) writeonly uniform highp uimage2D uImage[2];"
"void main()\n"
"{\n"
" imageStore(uImage[0], ivec2(gl_LocalInvocationIndex, 0), uvec4(100, 0, 0, 0));"
" imageStore(uImage[1], ivec2(gl_LocalInvocationIndex, 1), uvec4(100, 0, 0, 0));"
"}\n";
ANGLE_GL_COMPUTE_PROGRAM(program, csSource);
glUseProgram(program.get());
constexpr int kTextureWidth = 4, kTextureHeight = 2;
GLuint inputValues[] = {200, 200, 200, 200, 200, 200, 200, 200};
glBindTexture(GL_TEXTURE_2D, mTexture);
glTexStorage2D(GL_TEXTURE_2D, 1, GL_R32UI, kTextureWidth, kTextureHeight);
glTexSubImage2D(GL_TEXTURE_2D, 0, 0, 0, kTextureWidth, kTextureHeight, GL_RED_INTEGER,
GL_UNSIGNED_INT, inputValues);
EXPECT_GL_NO_ERROR();
glBindImageTexture(0, mTexture, 0, GL_FALSE, 0, GL_WRITE_ONLY, GL_R32UI);
glDispatchCompute(1, 1, 1);
EXPECT_GL_NO_ERROR();
glUseProgram(0);
glBindFramebuffer(GL_READ_FRAMEBUFFER, mFramebuffer);
glFramebufferTexture2D(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, mTexture, 0);
GLuint outputValues[8];
glReadPixels(0, 0, kTextureWidth, kTextureHeight, GL_RED_INTEGER, GL_UNSIGNED_INT,
outputValues);
EXPECT_GL_NO_ERROR();
GLuint expectedValue = 100;
for (int i = 0; i < kTextureWidth * kTextureHeight; i++)
{
EXPECT_EQ(expectedValue, outputValues[i]);
}
}
// Check that it is not possible to create a compute shader when the context does not support ES // Check that it is not possible to create a compute shader when the context does not support ES
// 3.10 // 3.10
TEST_P(ComputeShaderTestES3, NotSupported) TEST_P(ComputeShaderTestES3, NotSupported)
......
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