Commit cc2ed612 by Jamie Madill Committed by Commit Bot

Fix non-square matrix uniform packing.

This was broken in two places: the register count was using the non- transposed matrix row count. The block layout encoder was also not set to transpose matrices, which was causing incorrect packing info to be calculated in link. BUG=angleproject:1923 Change-Id: I89094aa116fad4bda15f018498f8637520f12bd4 Reviewed-on: https://chromium-review.googlesource.com/454876 Commit-Queue: Jamie Madill <jmadill@chromium.org> Reviewed-by: 's avatarGeoff Lang <geofflang@chromium.org> Reviewed-by: 's avatarOlli Etuaho <oetuaho@nvidia.com>
parent 36e120ef
...@@ -15,8 +15,8 @@ ...@@ -15,8 +15,8 @@
namespace sh namespace sh
{ {
HLSLBlockEncoder::HLSLBlockEncoder(HLSLBlockEncoderStrategy strategy) HLSLBlockEncoder::HLSLBlockEncoder(HLSLBlockEncoderStrategy strategy, bool transposeMatrices)
: mEncoderStrategy(strategy), mTransposeMatrices(false) : mEncoderStrategy(strategy), mTransposeMatrices(transposeMatrices)
{ {
} }
...@@ -154,25 +154,13 @@ void HLSLVariableRegisterCount(const ShaderVarType &variable, HLSLBlockEncoder * ...@@ -154,25 +154,13 @@ void HLSLVariableRegisterCount(const ShaderVarType &variable, HLSLBlockEncoder *
} }
} }
unsigned int HLSLVariableRegisterCount(const Varying &variable, bool transposeMatrices)
{
HLSLBlockEncoder encoder(HLSLBlockEncoder::ENCODE_PACKED);
encoder.setTransposeMatrices(transposeMatrices);
HLSLVariableRegisterCount(variable, &encoder);
const size_t registerBytes = (encoder.BytesPerComponent * encoder.ComponentsPerRegister);
return static_cast<unsigned int>(rx::roundUp<size_t>(encoder.getBlockSize(), registerBytes) /
registerBytes);
}
unsigned int HLSLVariableRegisterCount(const Uniform &variable, ShShaderOutput outputType) unsigned int HLSLVariableRegisterCount(const Uniform &variable, ShShaderOutput outputType)
{ {
HLSLBlockEncoder encoder(HLSLBlockEncoder::GetStrategyFor(outputType)); HLSLBlockEncoder encoder(HLSLBlockEncoder::GetStrategyFor(outputType), true);
encoder.setTransposeMatrices(true);
HLSLVariableRegisterCount(variable, &encoder); HLSLVariableRegisterCount(variable, &encoder);
const size_t registerBytes = (encoder.BytesPerComponent * encoder.ComponentsPerRegister); const size_t registerBytes = (encoder.BytesPerComponent * encoder.ComponentsPerRegister);
return static_cast<unsigned int>(rx::roundUp<size_t>(encoder.getBlockSize(), registerBytes) / return static_cast<unsigned int>(rx::roundUp<size_t>(encoder.getBlockSize(), registerBytes) /
registerBytes); registerBytes);
} }
} } // namespace sh
...@@ -33,14 +33,13 @@ class HLSLBlockEncoder : public BlockLayoutEncoder ...@@ -33,14 +33,13 @@ class HLSLBlockEncoder : public BlockLayoutEncoder
ENCODE_LOOSE ENCODE_LOOSE
}; };
HLSLBlockEncoder(HLSLBlockEncoderStrategy strategy); HLSLBlockEncoder(HLSLBlockEncoderStrategy strategy, bool transposeMatrices);
virtual void enterAggregateType(); virtual void enterAggregateType();
virtual void exitAggregateType(); virtual void exitAggregateType();
void skipRegisters(unsigned int numRegisters); void skipRegisters(unsigned int numRegisters);
bool isPacked() const { return mEncoderStrategy == ENCODE_PACKED; } bool isPacked() const { return mEncoderStrategy == ENCODE_PACKED; }
void setTransposeMatrices(bool enabled) { mTransposeMatrices = enabled; }
static HLSLBlockEncoderStrategy GetStrategyFor(ShShaderOutput outputType); static HLSLBlockEncoderStrategy GetStrategyFor(ShShaderOutput outputType);
...@@ -61,10 +60,8 @@ class HLSLBlockEncoder : public BlockLayoutEncoder ...@@ -61,10 +60,8 @@ class HLSLBlockEncoder : public BlockLayoutEncoder
}; };
// This method returns the number of used registers for a ShaderVariable. It is dependent on the // This method returns the number of used registers for a ShaderVariable. It is dependent on the
// HLSLBlockEncoder // HLSLBlockEncoder class to count the number of used registers in a struct (which are individually
// class to count the number of used registers in a struct (which are individually packed according // packed according to the same rules).
// to the same rules).
unsigned int HLSLVariableRegisterCount(const Varying &variable, bool transposeMatrices);
unsigned int HLSLVariableRegisterCount(const Uniform &variable, ShShaderOutput outputType); unsigned int HLSLVariableRegisterCount(const Uniform &variable, ShShaderOutput outputType);
} }
......
...@@ -260,7 +260,7 @@ D3DUniform::D3DUniform(GLenum typeIn, ...@@ -260,7 +260,7 @@ D3DUniform::D3DUniform(GLenum typeIn,
data = new uint8_t[bytes]; data = new uint8_t[bytes];
memset(data, 0, bytes); memset(data, 0, bytes);
// TODO(jmadill): is this correct with non-square matrices? // Use the row count as register count, will work for non-square matrices.
registerCount = gl::VariableRowCount(type) * elementCount(); registerCount = gl::VariableRowCount(type) * elementCount();
} }
} }
...@@ -1902,7 +1902,6 @@ void ProgramD3D::defineUniformsAndAssignRegisters() ...@@ -1902,7 +1902,6 @@ void ProgramD3D::defineUniformsAndAssignRegisters()
if (computeShader) if (computeShader)
{ {
for (const sh::Uniform &computeUniform : computeShader->getUniforms()) for (const sh::Uniform &computeUniform : computeShader->getUniforms())
{ {
if (computeUniform.staticUse) if (computeUniform.staticUse)
{ {
...@@ -1914,7 +1913,6 @@ void ProgramD3D::defineUniformsAndAssignRegisters() ...@@ -1914,7 +1913,6 @@ void ProgramD3D::defineUniformsAndAssignRegisters()
{ {
const gl::Shader *vertexShader = mState.getAttachedVertexShader(); const gl::Shader *vertexShader = mState.getAttachedVertexShader();
for (const sh::Uniform &vertexUniform : vertexShader->getUniforms()) for (const sh::Uniform &vertexUniform : vertexShader->getUniforms())
{ {
if (vertexUniform.staticUse) if (vertexUniform.staticUse)
{ {
...@@ -1962,7 +1960,7 @@ void ProgramD3D::defineUniformBase(const gl::Shader *shader, ...@@ -1962,7 +1960,7 @@ void ProgramD3D::defineUniformBase(const gl::Shader *shader,
unsigned int startRegister = shaderD3D->getUniformRegister(uniform.name); unsigned int startRegister = shaderD3D->getUniformRegister(uniform.name);
ShShaderOutput outputType = shaderD3D->getCompilerOutputType(); ShShaderOutput outputType = shaderD3D->getCompilerOutputType();
sh::HLSLBlockEncoder encoder(sh::HLSLBlockEncoder::GetStrategyFor(outputType)); sh::HLSLBlockEncoder encoder(sh::HLSLBlockEncoder::GetStrategyFor(outputType), true);
encoder.skipRegisters(startRegister); encoder.skipRegisters(startRegister);
defineUniform(shader->getType(), uniform, uniform.name, &encoder, uniformMap); defineUniform(shader->getType(), uniform, uniform.name, &encoder, uniformMap);
...@@ -2192,7 +2190,7 @@ size_t ProgramD3D::getUniformBlockInfo(const sh::InterfaceBlock &interfaceBlock) ...@@ -2192,7 +2190,7 @@ size_t ProgramD3D::getUniformBlockInfo(const sh::InterfaceBlock &interfaceBlock)
// define member uniforms // define member uniforms
sh::Std140BlockEncoder std140Encoder; sh::Std140BlockEncoder std140Encoder;
sh::HLSLBlockEncoder hlslEncoder(sh::HLSLBlockEncoder::ENCODE_PACKED); sh::HLSLBlockEncoder hlslEncoder(sh::HLSLBlockEncoder::ENCODE_PACKED, false);
sh::BlockLayoutEncoder *encoder = nullptr; sh::BlockLayoutEncoder *encoder = nullptr;
if (interfaceBlock.layout == sh::BLOCKLAYOUT_STANDARD) if (interfaceBlock.layout == sh::BLOCKLAYOUT_STANDARD)
......
...@@ -882,6 +882,49 @@ TEST_P(UniformTestES31, UnusedUniformArraysConflictingLocation) ...@@ -882,6 +882,49 @@ TEST_P(UniformTestES31, UnusedUniformArraysConflictingLocation)
EXPECT_EQ(0u, mProgram); EXPECT_EQ(0u, mProgram);
} }
// Test a uniform struct containing a non-square matrix and a boolean.
// Minimal test case for a bug revealed by dEQP tests.
TEST_P(UniformTestES3, StructWithNonSquareMatrixAndBool)
{
const std::string &vertShader =
"#version 300 es\n"
"precision highp float;\n"
"in highp vec4 a_position;\n"
"void main()\n"
"{\n"
" gl_Position = a_position;\n"
"}\n";
const std::string &fragShader =
"#version 300 es\n"
"precision highp float;\n"
"out highp vec4 my_color;\n"
"struct S\n"
"{\n"
" mat2x4 m;\n"
" bool b;\n"
"};\n"
"uniform S uni;\n"
"void main()\n"
"{\n"
" my_color = vec4(1.0);\n"
" if (!uni.b) { my_color.g = 0.0; }"
"}\n";
ANGLE_GL_PROGRAM(program, vertShader, fragShader);
glUseProgram(program.get());
GLint location = glGetUniformLocation(program.get(), "uni.b");
ASSERT_NE(-1, location);
glUniform1i(location, 1);
drawQuad(program.get(), "a_position", 0.0f);
ASSERT_GL_NO_ERROR();
EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::white);
}
// Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against. // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
ANGLE_INSTANTIATE_TEST(UniformTest, ANGLE_INSTANTIATE_TEST(UniformTest,
ES2_D3D9(), ES2_D3D9(),
......
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