Commit c97e900c by Jamie Madill Committed by Commit Bot

Revert "Vulkan: Fix issue in GlslWrapper and maxVaryingVectors calculation"

This reverts commit 1d882aaa. Sorry about this Luc. I think there was a missing suppression. You can even see a flaky failure in the CQ when you landed this - it seems the flakiness of the failure let it through. This is blocking another fix from landing which fixes the fact the CQ is on fire. The revert button is also missing from Gerrit which means this is a manual revert. Failures: https://ci.chromium.org/p/chromium/builders/luci.chromium.try/win_angle_rel_ng/361 Original CL Message: The layout needed to also have a component qualifier so that the registerColumn wouldn't be ignored. Bug: angleproject:2460 Bug: angleproject:2483 No-Try: True Change-Id: I84c38497fbda43d9defbc6d5d3ff0f9c133e6e46 Reviewed-on: https://chromium-review.googlesource.com/1015323Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org>
parent aed1b56a
...@@ -108,12 +108,11 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext, ...@@ -108,12 +108,11 @@ gl::LinkResult GlslangWrapper::linkProgram(const gl::Context *glContext,
} }
// Assign varying locations. // Assign varying locations.
// TODO(jmadill): This might need to be redone.
for (const auto &varyingReg : resources.varyingPacking.getRegisterList()) for (const auto &varyingReg : resources.varyingPacking.getRegisterList())
{ {
const auto &varying = *varyingReg.packedVarying; const auto &varying = *varyingReg.packedVarying;
std::string locationString = "location = " + Str(varyingReg.registerRow);
std::string locationString = "location = " + Str(varyingReg.registerRow) +
", component = " + Str(varyingReg.registerColumn);
InsertLayoutSpecifierString(&vertexSource, varying.varying->name, locationString); InsertLayoutSpecifierString(&vertexSource, varying.varying->name, locationString);
InsertLayoutSpecifierString(&fragmentSource, varying.varying->name, locationString); InsertLayoutSpecifierString(&fragmentSource, varying.varying->name, locationString);
} }
......
...@@ -121,11 +121,11 @@ void GenerateCaps(const VkPhysicalDeviceProperties &physicalDeviceProperties, ...@@ -121,11 +121,11 @@ void GenerateCaps(const VkPhysicalDeviceProperties &physicalDeviceProperties,
outCaps->maxVertexTextureImageUnits = outCaps->maxVertexTextureImageUnits =
physicalDeviceProperties.limits.maxPerStageDescriptorSamplers; physicalDeviceProperties.limits.maxPerStageDescriptorSamplers;
// The max vertex output components should not include gl_Position. // The max vertex output components includes the reserved varyings like gl_PointSize,
// The gles2.0 section 2.10 states that "gl_Position is not a varying variable and does // gl_PointCoord, gl_Position, and gl_FragColor. On a vertex shader, you can only have
// not count against this limit.", but the Vulkan spec has no such mention in its Built-in // gl_PointCoord and gl_Position, and on a fragment shader you can have gl_PointSize and
// vars section. It is implicit that we need to actually reserve it for Vulkan in that case. // gl_FragColor, so the reserved varying count is always 2 at most.
constexpr GLint kReservedVaryingCount = 1; constexpr GLint kReservedVaryingCount = 2;
outCaps->maxVaryingVectors = outCaps->maxVaryingVectors =
(physicalDeviceProperties.limits.maxVertexOutputComponents / 4) - kReservedVaryingCount; (physicalDeviceProperties.limits.maxVertexOutputComponents / 4) - kReservedVaryingCount;
outCaps->maxVertexOutputComponents = outCaps->maxVaryingVectors * 4; outCaps->maxVertexOutputComponents = outCaps->maxVaryingVectors * 4;
......
...@@ -1305,12 +1305,6 @@ TEST_P(GLSLTest, MaxVaryingVec4) ...@@ -1305,12 +1305,6 @@ TEST_P(GLSLTest, MaxVaryingVec4)
// Verify we can pack registers with one builtin varying. // Verify we can pack registers with one builtin varying.
TEST_P(GLSLTest, MaxVaryingVec4_OneBuiltin) TEST_P(GLSLTest, MaxVaryingVec4_OneBuiltin)
{ {
// TODO(lucferron): This test is failing on AMD Windows Vulkan only with an access violation
// error. It looks like we would need to reserve one more varying in the maxVaryingVectors
// caps for this one.
// http://anglebug.com/2483
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD() && IsWindows());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1321,12 +1315,6 @@ TEST_P(GLSLTest, MaxVaryingVec4_OneBuiltin) ...@@ -1321,12 +1315,6 @@ TEST_P(GLSLTest, MaxVaryingVec4_OneBuiltin)
// Verify we can pack registers with two builtin varyings. // Verify we can pack registers with two builtin varyings.
TEST_P(GLSLTest, MaxVaryingVec4_TwoBuiltins) TEST_P(GLSLTest, MaxVaryingVec4_TwoBuiltins)
{ {
// TODO(lucferron): This test is failing on AMD Windows Vulkan only with an access violation
// error. It looks like we would need to reserve one more varying in the maxVaryingVectors
// caps for this one.
// http://anglebug.com/2483
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD() && IsWindows());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1337,12 +1325,6 @@ TEST_P(GLSLTest, MaxVaryingVec4_TwoBuiltins) ...@@ -1337,12 +1325,6 @@ TEST_P(GLSLTest, MaxVaryingVec4_TwoBuiltins)
// Verify we can pack registers with three builtin varyings. // Verify we can pack registers with three builtin varyings.
TEST_P(GLSLTest, MaxVaryingVec4_ThreeBuiltins) TEST_P(GLSLTest, MaxVaryingVec4_ThreeBuiltins)
{ {
// TODO(lucferron): This test is failing on AMD Windows Vulkan only with an access violation
// error. It looks like we would need to reserve one more varying in the maxVaryingVectors
// caps for this one.
// http://anglebug.com/2483
ANGLE_SKIP_TEST_IF(IsVulkan() && IsAMD() && IsWindows());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1371,6 +1353,10 @@ TEST_P(GLSLTest, MaxVaryingsSpecialCases) ...@@ -1371,6 +1353,10 @@ TEST_P(GLSLTest, MaxVaryingsSpecialCases)
// rather than total register use. // rather than total register use.
TEST_P(GLSLTest, MaxMinusTwoVaryingVec2PlusOneSpecialVariable) TEST_P(GLSLTest, MaxMinusTwoVaryingVec2PlusOneSpecialVariable)
{ {
// TODO(lucferron): Root cause and fix it. Looks like a location is used twice (loc = 31).
// http://anglebug.com/2460
ANGLE_SKIP_TEST_IF(IsVulkan());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1397,6 +1383,11 @@ TEST_P(GLSLTest, MaxVaryingVec3Array) ...@@ -1397,6 +1383,11 @@ TEST_P(GLSLTest, MaxVaryingVec3Array)
// Only fails on D3D9 because of packing limitations. // Only fails on D3D9 because of packing limitations.
TEST_P(GLSLTest, MaxVaryingVec3AndOneFloat) TEST_P(GLSLTest, MaxVaryingVec3AndOneFloat)
{ {
// TODO(lucferron): Root cause and fix it.
// ERROR: 0:62: 'location' : overlapping use of location x
// http://anglebug.com/2460
ANGLE_SKIP_TEST_IF(IsVulkan());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1406,6 +1397,11 @@ TEST_P(GLSLTest, MaxVaryingVec3AndOneFloat) ...@@ -1406,6 +1397,11 @@ TEST_P(GLSLTest, MaxVaryingVec3AndOneFloat)
// Only fails on D3D9 because of packing limitations. // Only fails on D3D9 because of packing limitations.
TEST_P(GLSLTest, MaxVaryingVec3ArrayAndOneFloatArray) TEST_P(GLSLTest, MaxVaryingVec3ArrayAndOneFloatArray)
{ {
// TODO(lucferron): Root cause and fix it.
// ERROR: 0:62: 'location' : overlapping use of location x
// http://anglebug.com/2460
ANGLE_SKIP_TEST_IF(IsVulkan());
GLint maxVaryings = 0; GLint maxVaryings = 0;
glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings); glGetIntegerv(GL_MAX_VARYING_VECTORS, &maxVaryings);
...@@ -1415,6 +1411,11 @@ TEST_P(GLSLTest, MaxVaryingVec3ArrayAndOneFloatArray) ...@@ -1415,6 +1411,11 @@ TEST_P(GLSLTest, MaxVaryingVec3ArrayAndOneFloatArray)
// Only fails on D3D9 because of packing limitations. // Only fails on D3D9 because of packing limitations.
TEST_P(GLSLTest, TwiceMaxVaryingVec2) TEST_P(GLSLTest, TwiceMaxVaryingVec2)
{ {
// TODO(lucferron): Root cause and fix it.
// ERROR: 0:62: 'location' : overlapping use of location x
// http://anglebug.com/2460
ANGLE_SKIP_TEST_IF(IsVulkan());
// TODO(geofflang): Figure out why this fails on NVIDIA's GLES driver // TODO(geofflang): Figure out why this fails on NVIDIA's GLES driver
ANGLE_SKIP_TEST_IF(IsNVIDIA() && IsOpenGLES()); ANGLE_SKIP_TEST_IF(IsNVIDIA() && IsOpenGLES());
...@@ -1431,6 +1432,11 @@ TEST_P(GLSLTest, TwiceMaxVaryingVec2) ...@@ -1431,6 +1432,11 @@ TEST_P(GLSLTest, TwiceMaxVaryingVec2)
// Disabled because of a failure in D3D9 // Disabled because of a failure in D3D9
TEST_P(GLSLTest, MaxVaryingVec2Arrays) TEST_P(GLSLTest, MaxVaryingVec2Arrays)
{ {
// TODO(lucferron): Root cause and fix it.
// ERROR: 0:62: 'location' : overlapping use of location x
// http://anglebug.com/2460
ANGLE_SKIP_TEST_IF(IsVulkan());
ANGLE_SKIP_TEST_IF(IsD3DSM3()); ANGLE_SKIP_TEST_IF(IsD3DSM3());
// TODO(geofflang): Figure out why this fails on NVIDIA's GLES driver // TODO(geofflang): Figure out why this fails on NVIDIA's GLES driver
......
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