Commit 4cdbdf90 by Mohan Maiya Committed by Commit Bot

Vulkan: Assign XFB location first then assign varying location

Varying location is determined based on XFB location. If the transform feedback stage is Vertex, there is no problem. But if the transform feedback stage is GS or TES, previous implementation can cause shader interface mismatch. Bug: angleproject:5557 Test: KHR-GLES32.core.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in Change-Id: I1ecc7913a178c9e674307c528d1bdf13aabcb665 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2784713Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Mohan Maiya <m.maiya@samsung.com>
parent def0aa27
...@@ -240,50 +240,6 @@ void AssignTransformFeedbackEmulationBindings(gl::ShaderType shaderType, ...@@ -240,50 +240,6 @@ void AssignTransformFeedbackEmulationBindings(gl::ShaderType shaderType,
} }
} }
void AssignTransformFeedbackExtensionLocations(gl::ShaderType shaderType,
const gl::ProgramState &programState,
bool isTransformFeedbackStage,
GlslangProgramInterfaceInfo *programInterfaceInfo,
ShaderInterfaceVariableInfoMap *variableInfoMapOut)
{
// The only varying that requires additional resources is gl_Position, as it's indirectly
// captured through ANGLEXfbPosition.
const std::vector<gl::TransformFeedbackVarying> &tfVaryings =
programState.getLinkedTransformFeedbackVaryings();
bool capturesPosition = false;
if (isTransformFeedbackStage)
{
for (uint32_t varyingIndex = 0; varyingIndex < tfVaryings.size(); ++varyingIndex)
{
const gl::TransformFeedbackVarying &tfVarying = tfVaryings[varyingIndex];
const std::string &tfVaryingName = tfVarying.mappedName;
if (tfVaryingName == "gl_Position")
{
ASSERT(tfVarying.isBuiltIn());
capturesPosition = true;
break;
}
}
}
if (capturesPosition)
{
AddLocationInfo(variableInfoMapOut, shaderType, sh::vk::kXfbExtensionPositionOutName,
programInterfaceInfo->locationsUsedForXfbExtension, 0, 0, 0);
++programInterfaceInfo->locationsUsedForXfbExtension;
}
else
{
// Make sure this varying is removed from the other stages, or if position is not captured
// at all.
variableInfoMapOut->add(shaderType, sh::vk::kXfbExtensionPositionOutName);
}
}
bool IsFirstRegisterOfVarying(const gl::PackedVaryingRegister &varyingReg, bool allowFields) bool IsFirstRegisterOfVarying(const gl::PackedVaryingRegister &varyingReg, bool allowFields)
{ {
const gl::PackedVarying &varying = *varyingReg.packedVarying; const gl::PackedVarying &varying = *varyingReg.packedVarying;
...@@ -4798,15 +4754,6 @@ void GlslangAssignLocations(const GlslangSourceOptions &options, ...@@ -4798,15 +4754,6 @@ void GlslangAssignLocations(const GlslangSourceOptions &options,
const gl::VaryingPacking &inputPacking = varyingPacking.getInputPacking(shaderType); const gl::VaryingPacking &inputPacking = varyingPacking.getInputPacking(shaderType);
const gl::VaryingPacking &outputPacking = varyingPacking.getOutputPacking(shaderType); const gl::VaryingPacking &outputPacking = varyingPacking.getOutputPacking(shaderType);
// Assign location to varyings generated for transform feedback capture
if (options.supportsTransformFeedbackExtension &&
gl::ShaderTypeSupportsTransformFeedback(shaderType))
{
AssignTransformFeedbackExtensionLocations(shaderType, programState,
isTransformFeedbackStage,
programInterfaceInfo, variableInfoMapOut);
}
// Assign varying locations. // Assign varying locations.
if (shaderType != gl::ShaderType::Vertex) if (shaderType != gl::ShaderType::Vertex)
{ {
...@@ -4849,6 +4796,50 @@ void GlslangAssignLocations(const GlslangSourceOptions &options, ...@@ -4849,6 +4796,50 @@ void GlslangAssignLocations(const GlslangSourceOptions &options,
} }
} }
void GlslangAssignTransformFeedbackLocations(gl::ShaderType shaderType,
const gl::ProgramState &programState,
bool isTransformFeedbackStage,
GlslangProgramInterfaceInfo *programInterfaceInfo,
ShaderInterfaceVariableInfoMap *variableInfoMapOut)
{
// The only varying that requires additional resources is gl_Position, as it's indirectly
// captured through ANGLEXfbPosition.
const std::vector<gl::TransformFeedbackVarying> &tfVaryings =
programState.getLinkedTransformFeedbackVaryings();
bool capturesPosition = false;
if (isTransformFeedbackStage)
{
for (uint32_t varyingIndex = 0; varyingIndex < tfVaryings.size(); ++varyingIndex)
{
const gl::TransformFeedbackVarying &tfVarying = tfVaryings[varyingIndex];
const std::string &tfVaryingName = tfVarying.mappedName;
if (tfVaryingName == "gl_Position")
{
ASSERT(tfVarying.isBuiltIn());
capturesPosition = true;
break;
}
}
}
if (capturesPosition)
{
AddLocationInfo(variableInfoMapOut, shaderType, sh::vk::kXfbExtensionPositionOutName,
programInterfaceInfo->locationsUsedForXfbExtension, 0, 0, 0);
++programInterfaceInfo->locationsUsedForXfbExtension;
}
else
{
// Make sure this varying is removed from the other stages, or if position is not captured
// at all.
variableInfoMapOut->add(shaderType, sh::vk::kXfbExtensionPositionOutName);
}
}
void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options, void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options,
const gl::ProgramState &programState, const gl::ProgramState &programState,
const gl::ProgramLinkedResources &resources, const gl::ProgramLinkedResources &resources,
...@@ -4865,6 +4856,21 @@ void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options, ...@@ -4865,6 +4856,21 @@ void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options,
gl::ShaderType xfbStage = programState.getAttachedTransformFeedbackStage(); gl::ShaderType xfbStage = programState.getAttachedTransformFeedbackStage();
gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum; gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
// This should be done before assigning varying location. Otherwise, We can encounter shader
// interface mismatching problem in case the transformFeedback stage is not Vertex stage.
for (const gl::ShaderType shaderType : programState.getExecutable().getLinkedShaderStages())
{
// Assign location to varyings generated for transform feedback capture
const bool isXfbStage =
shaderType == xfbStage && !programState.getLinkedTransformFeedbackVaryings().empty();
if (options.supportsTransformFeedbackExtension &&
gl::ShaderTypeSupportsTransformFeedback(shaderType))
{
GlslangAssignTransformFeedbackLocations(shaderType, programState, isXfbStage,
programInterfaceInfo, variableInfoMapOut);
}
}
for (const gl::ShaderType shaderType : programState.getExecutable().getLinkedShaderStages()) for (const gl::ShaderType shaderType : programState.getExecutable().getLinkedShaderStages())
{ {
const bool isXfbStage = const bool isXfbStage =
......
...@@ -177,6 +177,12 @@ void GlslangAssignLocations(const GlslangSourceOptions &options, ...@@ -177,6 +177,12 @@ void GlslangAssignLocations(const GlslangSourceOptions &options,
GlslangProgramInterfaceInfo *programInterfaceInfo, GlslangProgramInterfaceInfo *programInterfaceInfo,
ShaderInterfaceVariableInfoMap *variableInfoMapOut); ShaderInterfaceVariableInfoMap *variableInfoMapOut);
void GlslangAssignTransformFeedbackLocations(gl::ShaderType shaderType,
const gl::ProgramState &programState,
bool isTransformFeedbackStage,
GlslangProgramInterfaceInfo *programInterfaceInfo,
ShaderInterfaceVariableInfoMap *variableInfoMapOut);
// Retrieves the compiled SPIR-V code for each shader stage, and calls |GlslangAssignLocations|. // Retrieves the compiled SPIR-V code for each shader stage, and calls |GlslangAssignLocations|.
void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options, void GlslangGetShaderSpirvCode(const GlslangSourceOptions &options,
const gl::ProgramState &programState, const gl::ProgramState &programState,
......
...@@ -71,21 +71,35 @@ angle::Result ProgramPipelineVk::link(const gl::Context *glContext, ...@@ -71,21 +71,35 @@ angle::Result ProgramPipelineVk::link(const gl::Context *glContext,
// set/binding locations need to be re-assigned to their correct values. // set/binding locations need to be re-assigned to their correct values.
const gl::ShaderType linkedTransformFeedbackStage = const gl::ShaderType linkedTransformFeedbackStage =
glExecutable.getLinkedTransformFeedbackStage(); glExecutable.getLinkedTransformFeedbackStage();
gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
// This should be done before assigning varying location. Otherwise, We can encounter shader
// interface mismatching problem in case the transformFeedback stage is not Vertex stage.
for (const gl::ShaderType shaderType : glExecutable.getLinkedShaderStages()) for (const gl::ShaderType shaderType : glExecutable.getLinkedShaderStages())
{ {
gl::Program *glProgram = gl::Program *glProgram =
const_cast<gl::Program *>(glPipeline->getShaderProgram(shaderType)); const_cast<gl::Program *>(glPipeline->getShaderProgram(shaderType));
if (glProgram) if (glProgram)
{ {
// The program interface info must survive across shaders, except const bool isTransformFeedbackStage =
// for some program-specific values. shaderType == linkedTransformFeedbackStage &&
ProgramVk *programVk = vk::GetImpl(glProgram); !glProgram->getState().getLinkedTransformFeedbackVaryings().empty();
const GlslangProgramInterfaceInfo &programProgramInterfaceInfo = if (options.supportsTransformFeedbackExtension &&
programVk->getGlslangProgramInterfaceInfo(); gl::ShaderTypeSupportsTransformFeedback(shaderType))
glslangProgramInterfaceInfo.locationsUsedForXfbExtension = {
programProgramInterfaceInfo.locationsUsedForXfbExtension; GlslangAssignTransformFeedbackLocations(
shaderType, glProgram->getState(), isTransformFeedbackStage,
&glslangProgramInterfaceInfo, &mExecutable.mVariableInfoMap);
}
}
}
gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
for (const gl::ShaderType shaderType : glExecutable.getLinkedShaderStages())
{
gl::Program *glProgram =
const_cast<gl::Program *>(glPipeline->getShaderProgram(shaderType));
if (glProgram)
{
const bool isTransformFeedbackStage = const bool isTransformFeedbackStage =
shaderType == linkedTransformFeedbackStage && shaderType == linkedTransformFeedbackStage &&
!glProgram->getState().getLinkedTransformFeedbackVaryings().empty(); !glProgram->getState().getLinkedTransformFeedbackVaryings().empty();
......
...@@ -60,7 +60,6 @@ ...@@ -60,7 +60,6 @@
5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.tessellation_shader_triangles_tessellation.degenerate_triangle = FAIL 5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.tessellation_shader_triangles_tessellation.degenerate_triangle = FAIL
// Missing names in mVariableInfo map (fail on ASSERT in SPIR-V transformer) // Missing names in mVariableInfo map (fail on ASSERT in SPIR-V transformer)
5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.single.xfb_captures_data_from_correct_stage = SKIP 5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.single.xfb_captures_data_from_correct_stage = SKIP
5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.tessellation_control_to_tessellation_evaluation.gl_in = SKIP
// Translator validation bugs // Translator validation bugs
5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.single.max_patch_vertices = FAIL 5557 VULKAN WIN : KHR-GLES32.core.tessellation_shader.single.max_patch_vertices = FAIL
// Bug in front-end where builtins are not correctly marked active in every stage // Bug in front-end where builtins are not correctly marked active in every stage
......
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