Skip to content

Commit

Permalink
Merge pull request #1235 from KhronosGroup/patch-input-array-fix
Browse files Browse the repository at this point in the history
GLSL: Fix array of input patch variables.
  • Loading branch information
HansKristian-Work authored Dec 10, 2019
2 parents 363035c + 7c1e34f commit f912c32
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ layout(vertices = 1) out;
in gl_PerVertex
{
vec4 gl_Position;
} gl_in[gl_MaxPatchVertices];
} gl_in[];

out gl_PerVertex
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ layout(triangles, cw, fractional_even_spacing) in;
in gl_PerVertex
{
vec4 gl_Position;
} gl_in[gl_MaxPatchVertices];
} gl_in[];

out gl_PerVertex
{
Expand Down
10 changes: 10 additions & 0 deletions reference/opt/shaders/tese/patch-input-array.tese
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#version 450
layout(quads, ccw, equal_spacing) in;

layout(location = 0) patch in float P[4];

void main()
{
gl_Position = vec4(P[0], P[1], P[2], P[3]);
}

2 changes: 1 addition & 1 deletion reference/shaders/desktop-only/tesc/basic.desktop.sso.tesc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ layout(vertices = 1) out;
in gl_PerVertex
{
vec4 gl_Position;
} gl_in[gl_MaxPatchVertices];
} gl_in[];

out gl_PerVertex
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ layout(triangles, cw, fractional_even_spacing) in;
in gl_PerVertex
{
vec4 gl_Position;
} gl_in[gl_MaxPatchVertices];
} gl_in[];

out gl_PerVertex
{
Expand Down
10 changes: 10 additions & 0 deletions reference/shaders/tese/patch-input-array.tese
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#version 450
layout(quads, ccw, equal_spacing) in;

layout(location = 0) patch in float P[4];

void main()
{
gl_Position = vec4(P[0], P[1], P[2], P[3]);
}

9 changes: 9 additions & 0 deletions shaders/tese/patch-input-array.tese
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#version 450

layout(quads) in;
layout(location = 0) patch in float P[4];

void main()
{
gl_Position = vec4(P[0], P[1], P[2], P[3]);
}
37 changes: 26 additions & 11 deletions spirv_glsl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2104,9 +2104,32 @@ void CompilerGLSL::emit_interface_block(const SPIRVariable &var)
else
{
add_resource_name(var.self);

// Tessellation control and evaluation shaders must have either gl_MaxPatchVertices or unsized arrays for input arrays.
// Opt for unsized as it's the more "correct" variant to use.
bool control_point_input_array = type.storage == StorageClassInput &&
!type.array.empty() && !has_decoration(var.self, DecorationPatch) &&
(get_entry_point().model == ExecutionModelTessellationControl ||
get_entry_point().model == ExecutionModelTessellationEvaluation);

uint32_t old_array_size = 0;
bool old_array_size_literal = true;

if (control_point_input_array)
{
swap(type.array.back(), old_array_size);
swap(type.array_size_literal.back(), old_array_size_literal);
}

statement(layout_for_variable(var), to_qualifiers_glsl(var.self),
variable_decl(type, to_name(var.self), var.self), ";");

if (control_point_input_array)
{
swap(type.array.back(), old_array_size);
swap(type.array_size_literal.back(), old_array_size_literal);
}

// If a StorageClassOutput variable has an initializer, we need to initialize it in main().
if (var.storage == StorageClassOutput && var.initializer)
{
Expand Down Expand Up @@ -2478,7 +2501,6 @@ void CompilerGLSL::emit_declared_builtin_block(StorageClass storage, ExecutionMo
if (emitted_builtins.get(BuiltInCullDistance))
statement("float gl_CullDistance[", cull_distance_size, "];");

bool tessellation = model == ExecutionModelTessellationEvaluation || model == ExecutionModelTessellationControl;
if (builtin_array)
{
// Make sure the array has a supported name in the code.
Expand All @@ -2490,7 +2512,7 @@ void CompilerGLSL::emit_declared_builtin_block(StorageClass storage, ExecutionMo
if (model == ExecutionModelTessellationControl && storage == StorageClassOutput)
end_scope_decl(join(to_name(block_var->self), "[", get_entry_point().output_vertices, "]"));
else
end_scope_decl(join(to_name(block_var->self), tessellation ? "[gl_MaxPatchVertices]" : "[]"));
end_scope_decl(join(to_name(block_var->self), "[]"));
}
else
end_scope_decl();
Expand Down Expand Up @@ -10794,14 +10816,6 @@ string CompilerGLSL::to_array_size(const SPIRType &type, uint32_t index)
{
assert(type.array.size() == type.array_size_literal.size());

// Tessellation control and evaluation shaders must have either gl_MaxPatchVertices or unsized arrays for input arrays.
// Opt for unsized as it's the more "correct" variant to use.
if (type.storage == StorageClassInput &&
(get_entry_point().model == ExecutionModelTessellationControl ||
get_entry_point().model == ExecutionModelTessellationEvaluation) &&
index == uint32_t(type.array.size() - 1))
return "";

auto &size = type.array[index];
if (!type.array_size_literal[index])
return to_expression(size);
Expand Down Expand Up @@ -12795,13 +12809,14 @@ void CompilerGLSL::unroll_array_from_complex_load(uint32_t target_id, uint32_t s
auto builtin = BuiltIn(get_decoration(var->self, DecorationBuiltIn));
bool is_builtin = is_builtin_variable(*var) && (builtin == BuiltInPointSize || builtin == BuiltInPosition);
bool is_tess = is_tessellation_shader();
bool is_patch = has_decoration(var->self, DecorationPatch);

// Tessellation input arrays are special in that they are unsized, so we cannot directly copy from it.
// We must unroll the array load.
// For builtins, we couldn't catch this case normally,
// because this is resolved in the OpAccessChain in most cases.
// If we load the entire array, we have no choice but to unroll here.
if (is_builtin || is_tess)
if (!is_patch && (is_builtin || is_tess))
{
auto new_expr = join("_", target_id, "_unrolled");
statement(variable_decl(type, new_expr, target_id), ";");
Expand Down

0 comments on commit f912c32

Please sign in to comment.