Commit feb3b6cc by Shahbaz Youssefi Committed by Commit Bot

Vulkan: Remove row->col major shader transformation

This was done based on the incorrect assumption that Vulkan GLSL doesn't allow layout qualifiers on interface block fields. This was due to glslang compile failures in some shaders that included mixed row- and column-major fields in interface blocks. However, the failures were only in the case the interface block is inactive, in which case glslang wrapper previously replaced the layout/qualifier of the interface block with |struct|, which left the shader with an unused struct definition with fields that have layout qualifiers; an invalid shader. The change introduced in https://chromium-review.googlesource.com/c/angle/angle/+/1951523 removes inactive shader interface declarations. The above scenario thus never occurs, rendering the row- to column-major transformation unnecessary. Bug: angleproject:3443 Change-Id: Ice34a0fc6e047b79a4d44f04b730ec59bdfafe33 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1961098Reviewed-by: 's avatarJonah Ryan-Davis <jonahr@google.com> Reviewed-by: 's avatarCody Northrop <cnorthrop@google.com> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent b09d46cc
...@@ -43,7 +43,7 @@ class TOutputGLSLBase : public TIntermTraverser ...@@ -43,7 +43,7 @@ class TOutputGLSLBase : public TIntermTraverser
std::string getCommonLayoutQualifiers(TIntermTyped *variable); std::string getCommonLayoutQualifiers(TIntermTyped *variable);
std::string getMemoryQualifiers(const TType &type); std::string getMemoryQualifiers(const TType &type);
virtual void writeLayoutQualifier(TIntermTyped *variable); virtual void writeLayoutQualifier(TIntermTyped *variable);
virtual void writeFieldLayoutQualifier(const TField *field); void writeFieldLayoutQualifier(const TField *field);
void writeInvariantQualifier(const TType &type); void writeInvariantQualifier(const TType &type);
void writePreciseQualifier(const TType &type); void writePreciseQualifier(const TType &type);
virtual void writeVariableType(const TType &type, virtual void writeVariableType(const TType &type,
......
...@@ -53,7 +53,8 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -53,7 +53,8 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
return; return;
} }
TInfoSinkBase &out = objSink(); TInfoSinkBase &out = objSink();
const TLayoutQualifier &layoutQualifier = type.getLayoutQualifier();
// This isn't super clean, but it gets the job done. // This isn't super clean, but it gets the job done.
// See corresponding code in glslang_wrapper_utils.cpp. // See corresponding code in glslang_wrapper_utils.cpp.
...@@ -87,12 +88,12 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -87,12 +88,12 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
{ {
blockStorage = getBlockStorageString(storage); blockStorage = getBlockStorageString(storage);
} }
}
// We expect all interface blocks to have been transformed to column major, so we don't // Specify matrix packing if necessary.
// specify the packing. Any remaining interface block qualified with row_major shouldn't if (layoutQualifier.matrixPacking != EmpUnspecified)
// have any matrices inside. {
ASSERT(type.getLayoutQualifier().matrixPacking != EmpRowMajor || matrixPacking = getMatrixPackingString(layoutQualifier.matrixPacking);
!interfaceBlock->containsMatrices());
} }
if (needsCustomLayout) if (needsCustomLayout)
...@@ -131,14 +132,6 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable) ...@@ -131,14 +132,6 @@ void TOutputVulkanGLSL::writeLayoutQualifier(TIntermTyped *variable)
} }
} }
void TOutputVulkanGLSL::writeFieldLayoutQualifier(const TField *field)
{
// We expect all interface blocks to have been transformed to column major, as Vulkan GLSL
// doesn't allow layout qualifiers on interface block fields. Any remaining interface block
// qualified with row_major shouldn't have any matrices inside, so the qualifier can be
// dropped.
}
void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier, void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier,
const TType &type, const TType &type,
const TSymbol *symbol) const TSymbol *symbol)
......
...@@ -31,7 +31,6 @@ class TOutputVulkanGLSL : public TOutputGLSL ...@@ -31,7 +31,6 @@ class TOutputVulkanGLSL : public TOutputGLSL
protected: protected:
void writeLayoutQualifier(TIntermTyped *variable) override; void writeLayoutQualifier(TIntermTyped *variable) override;
void writeFieldLayoutQualifier(const TField *field) override;
void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol) override; void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol) override;
void writeVariableType(const TType &type, void writeVariableType(const TType &type,
const TSymbol *symbol, const TSymbol *symbol,
......
...@@ -18,12 +18,10 @@ ...@@ -18,12 +18,10 @@
#include "compiler/translator/OutputVulkanGLSL.h" #include "compiler/translator/OutputVulkanGLSL.h"
#include "compiler/translator/StaticType.h" #include "compiler/translator/StaticType.h"
#include "compiler/translator/tree_ops/NameEmbeddedUniformStructs.h" #include "compiler/translator/tree_ops/NameEmbeddedUniformStructs.h"
#include "compiler/translator/tree_ops/NameNamelessUniformBuffers.h"
#include "compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.h" #include "compiler/translator/tree_ops/RemoveInactiveInterfaceVariables.h"
#include "compiler/translator/tree_ops/RewriteAtomicCounters.h" #include "compiler/translator/tree_ops/RewriteAtomicCounters.h"
#include "compiler/translator/tree_ops/RewriteCubeMapSamplersAs2DArray.h" #include "compiler/translator/tree_ops/RewriteCubeMapSamplersAs2DArray.h"
#include "compiler/translator/tree_ops/RewriteDfdy.h" #include "compiler/translator/tree_ops/RewriteDfdy.h"
#include "compiler/translator/tree_ops/RewriteRowMajorMatrices.h"
#include "compiler/translator/tree_ops/RewriteStructSamplers.h" #include "compiler/translator/tree_ops/RewriteStructSamplers.h"
#include "compiler/translator/tree_util/BuiltIn.h" #include "compiler/translator/tree_util/BuiltIn.h"
#include "compiler/translator/tree_util/FindFunction.h" #include "compiler/translator/tree_util/FindFunction.h"
...@@ -779,25 +777,6 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root, ...@@ -779,25 +777,6 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
} }
} }
if (getShaderVersion() >= 300)
{
// Make sure every uniform buffer variable has a name. The following transformation relies
// on this.
if (!NameNamelessUniformBuffers(this, root, &getSymbolTable()))
{
return false;
}
// In GLES3+, matrices can be declared row- or column-major. Transform all to column-major
// as interface block field layout qualifiers are not allowed. This should be done after
// samplers are taken out of structs (as structs could be rewritten), but before uniforms
// are collected in a uniform buffer as they are handled especially.
if (!RewriteRowMajorMatrices(this, root, &getSymbolTable()))
{
return false;
}
}
if (defaultUniformCount > 0) if (defaultUniformCount > 0)
{ {
sink << "\n@@ LAYOUT-defaultUniforms(std140) @@ uniform defaultUniforms\n{\n"; sink << "\n@@ LAYOUT-defaultUniforms(std140) @@ uniform defaultUniforms\n{\n";
......
...@@ -7254,10 +7254,6 @@ void main() ...@@ -7254,10 +7254,6 @@ void main()
// Test that side effects respect the order of logical expression operands. // Test that side effects respect the order of logical expression operands.
TEST_P(GLSLTest_ES3, MixedRowAndColumnMajorMatrices_ReadSideEffectOrder) TEST_P(GLSLTest_ES3, MixedRowAndColumnMajorMatrices_ReadSideEffectOrder)
{ {
// IntermTraverser::insertStatementsInParentBlock that's used to move side effects does not
// respect the order of evaluation of logical expressions. http://anglebug.com/3829.
ANGLE_SKIP_TEST_IF(IsVulkan());
// http://anglebug.com/3837 // http://anglebug.com/3837
ANGLE_SKIP_TEST_IF(IsLinux() && IsIntel() && IsOpenGL()); ANGLE_SKIP_TEST_IF(IsLinux() && IsIntel() && IsOpenGL());
...@@ -7319,10 +7315,6 @@ void main() ...@@ -7319,10 +7315,6 @@ void main()
// Test that side effects respect short-circuit. // Test that side effects respect short-circuit.
TEST_P(GLSLTest_ES3, MixedRowAndColumnMajorMatrices_ReadSideEffectShortCircuit) TEST_P(GLSLTest_ES3, MixedRowAndColumnMajorMatrices_ReadSideEffectShortCircuit)
{ {
// IntermTraverser::insertStatementsInParentBlock that's used to move side effects does not
// respect short-circuiting in evaluation of logical expressions. http://anglebug.com/3829.
ANGLE_SKIP_TEST_IF(IsVulkan());
// Fails on Android: http://anglebug.com/3839 // Fails on Android: http://anglebug.com/3839
ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGL()); ANGLE_SKIP_TEST_IF(IsAndroid() && IsOpenGL());
...@@ -7595,6 +7587,9 @@ TEST_P(GLSLTest_ES31, MixedRowAndColumnMajorMatrices_WriteArrayOfArray) ...@@ -7595,6 +7587,9 @@ TEST_P(GLSLTest_ES31, MixedRowAndColumnMajorMatrices_WriteArrayOfArray)
// Fails on D3D due to mistranslation: http://anglebug.com/3841 // Fails on D3D due to mistranslation: http://anglebug.com/3841
ANGLE_SKIP_TEST_IF(IsD3D11()); ANGLE_SKIP_TEST_IF(IsD3D11());
// Fails compiling shader on Android/Vulkan. http://anglebug.com/4290
ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
constexpr char kFS[] = R"(#version 310 es constexpr char kFS[] = R"(#version 310 es
precision highp float; precision highp float;
out vec4 outColor; out vec4 outColor;
......
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