Remove the 'EMIT_OFFSCREEN_VERTEX' macro in PLS D3D and metal both have an opposite sense of Y when rendering to an offscreen texture. This knowledge used to be baked into the shaders, but that makes it difficult to correctly get triangle windings when we're using face culling. This PR instead moves this knowledge of the Y direction into variables by negating the Y viewport uniforms when necessary. This change also exposed an issue in minify.py where HLSL semantic names were getting collisions because they are NOT case sensitive. So this change also updates minify.py to not use lower case letters in "@exported" names. Diffs= 623766365 Remove the 'EMIT_OFFSCREEN_VERTEX' macro in PLS (#5615) Co-authored-by: Chris Dalton <99840794+csmartdalton@users.noreply.github.com>
diff --git a/.rive_head b/.rive_head index 799bb02..f735bcd 100644 --- a/.rive_head +++ b/.rive_head
@@ -1 +1 @@ -b176711307aaa8dade145024af8a29a32b48296a +6237663659882f2ee15582fd8c9e8c517fe66a2f
diff --git a/include/rive/pls/pls.hpp b/include/rive/pls/pls.hpp index 3edb100..279de98 100644 --- a/include/rive/pls/pls.hpp +++ b/include/rive/pls/pls.hpp
@@ -163,6 +163,8 @@ uint8_t pathIDGranularity = 1; // Workaround for precision issues. Determines how far apart we // space unique path IDs. bool avoidFlatVaryings = false; + bool invertOffscreenY = false; // Invert Y when drawing to offscreen render targets? (Gradient + // and tessellation textures.) }; // Per-flush shared uniforms used by all shaders. @@ -174,10 +176,12 @@ size_t renderTargetHeight, size_t gradTextureHeight, const PlatformFeatures& platformFeatures) : - inverseViewports(2.f / float4{static_cast<float>(complexGradientsHeight), - static_cast<float>(tessDataHeight), - static_cast<float>(renderTargetWidth), - static_cast<float>(renderTargetHeight)}), + inverseViewports( + (platformFeatures.invertOffscreenY ? float4{-2.f, -2.f, 2.f, 2.f} : float4(2.f)) / + float4{static_cast<float>(complexGradientsHeight), + static_cast<float>(tessDataHeight), + static_cast<float>(renderTargetWidth), + static_cast<float>(renderTargetHeight)}), gradTextureInverseHeight(1.f / static_cast<float>(gradTextureHeight)), pathIDGranularity(platformFeatures.pathIDGranularity) {}
diff --git a/renderer/d3d/pls_render_context_d3d.cpp b/renderer/d3d/pls_render_context_d3d.cpp index 40bc53c..2d5a770 100644 --- a/renderer/d3d/pls_render_context_d3d.cpp +++ b/renderer/d3d/pls_render_context_d3d.cpp
@@ -144,10 +144,20 @@ return blob; } +static PlatformFeatures platform_features_d3d() +{ + PlatformFeatures platformFeatures; + platformFeatures.invertOffscreenY = true; + return platformFeatures; +} + PLSRenderContextD3D::PLSRenderContextD3D(ComPtr<ID3D11Device> gpu, ComPtr<ID3D11DeviceContext> gpuContext, bool isIntel) : - PLSRenderContext(PlatformFeatures()), m_isIntel(isIntel), m_gpu(gpu), m_gpuContext(gpuContext) + PLSRenderContext(platform_features_d3d()), + m_isIntel(isIntel), + m_gpu(gpu), + m_gpuContext(gpuContext) { D3D11_RASTERIZER_DESC rasterDesc; rasterDesc.FillMode = D3D11_FILL_SOLID;
diff --git a/renderer/metal/pls_render_context_metal.mm b/renderer/metal/pls_render_context_metal.mm index ff1f01e..aee8c2a 100644 --- a/renderer/metal/pls_render_context_metal.mm +++ b/renderer/metal/pls_render_context_metal.mm
@@ -187,6 +187,7 @@ // It appears, so far, that we don't need to use flat interpolation for path IDs on any Apple // device, and it's faster not to. platformFeatures.avoidFlatVaryings = true; + platformFeatures.invertOffscreenY = true; return std::unique_ptr<PLSRenderContextMetal>( new PLSRenderContextMetal(platformFeatures, gpu, queue)); }
diff --git a/renderer/shaders/color_ramp.glsl b/renderer/shaders/color_ramp.glsl index 0d26ebf..d099f4d 100644 --- a/renderer/shaders/color_ramp.glsl +++ b/renderer/shaders/color_ramp.glsl
@@ -45,11 +45,11 @@ float y = float(@a_span.y) + ((_vertexID & 2) == 0 ? .0 : 1.); v_rampColor = unpackColorInt((_vertexID & 1) == 0 ? @a_span.z : @a_span.w); _pos.x = x * 2. - 1.; - _pos.y = y * uniforms.gradInverseViewportY - 1.; + _pos.y = y * uniforms.gradInverseViewportY - sign(uniforms.gradInverseViewportY); _pos.zw = float2(0, 1); VARYING_PACK(varyings, v_rampColor); - EMIT_OFFSCREEN_VERTEX(varyings, _pos); + EMIT_VERTEX(varyings, _pos); } #endif
diff --git a/renderer/shaders/glsl.glsl b/renderer/shaders/glsl.glsl index c1254a0..3945131 100644 --- a/renderer/shaders/glsl.glsl +++ b/renderer/shaders/glsl.glsl
@@ -244,10 +244,6 @@ } \ gl_Position = _pos; -#define EMIT_OFFSCREEN_VERTEX(varyings, _pos) \ - } \ - gl_Position = _pos; - #define FRAG_DATA_MAIN(DATA_TYPE, NAME, Varyings, varyings) \ out DATA_TYPE _fd; \ void main()
diff --git a/renderer/shaders/hlsl.glsl b/renderer/shaders/hlsl.glsl index 99003c9..afe51c4 100644 --- a/renderer/shaders/hlsl.glsl +++ b/renderer/shaders/hlsl.glsl
@@ -177,12 +177,6 @@ varyings._pos = _pos; \ return varyings; -#define EMIT_OFFSCREEN_VERTEX(varyings, _pos) \ - varyings._pos.xzw = _pos.xzw; \ - varyings._pos.y = -_pos.y; \ - return varyings; \ - } - #define FRAG_DATA_MAIN(DATA_TYPE, NAME, Varyings, varyings) \ DATA_TYPE NAME(Varyings varyings) : SV_Target \ {
diff --git a/renderer/shaders/metal.glsl b/renderer/shaders/metal.glsl index 8a147ea..022cbab 100644 --- a/renderer/shaders/metal.glsl +++ b/renderer/shaders/metal.glsl
@@ -159,12 +159,6 @@ varyings._pos = _pos; \ return varyings; -#define EMIT_OFFSCREEN_VERTEX(varyings, _pos) \ - varyings._pos.xzw = _pos.xzw; \ - varyings._pos.y = -_pos.y; \ - return varyings; \ - } - #define FRAG_DATA_MAIN(DATA_TYPE, NAME, Varyings, varyings) \ DATA_TYPE __attribute__((visibility("default"))) fragment NAME(Varyings varyings [[stage_in]]) \ {
diff --git a/renderer/shaders/minify.py b/renderer/shaders/minify.py index c427ec4..4c09129 100644 --- a/renderer/shaders/minify.py +++ b/renderer/shaders/minify.py
@@ -15,8 +15,8 @@ * Strip unused #defines. * Rename stpq and rgba swizzles to xyzw. * Rename variables. - - No new name includes the '_' character, so internal code can use '_' without fear of - renaming collisions. + - No new name begins with the '_' character, so internal code can begin names with '_' + without fear of renaming collisions. - GLSL keywords and builtins are not renamed. - Tokens beginning with '@' have their new name exported to a header file. - Tokens beginning with '$' are not renamed, with the exception of removing the leading '$'. @@ -301,20 +301,44 @@ def remove_leading_annotation(name): return name[1:] if name[0] == '@' or name[0] == '$' else name -# generates new identifier names to rewrite our variables. -# exclude '_' from our new names. Internal variables can use '_' to avoid naming collisions. -new_name_chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" -next_new_name_index = 0 -def generate_new_name(): - global next_new_name_index, new_name_chars - while True: - i = next_new_name_index - next_new_name_index = next_new_name_index + 1 - name = new_name_chars[0] if i == 0 else "" +# Generates new identifier names to rewrite our variables. +class NameGenerator: + def __init__(self, first_letter_chars, additional_letter_chars): + self.first_letter_chars = first_letter_chars + self.additional_letter_chars = additional_letter_chars + self.name_index = 0 + + def next_name(self): + i = self.name_index + # Generate the first character from 'self.first_letter_chars' + name = self.first_letter_chars[i % len(self.first_letter_chars)] + i = i // len(self.first_letter_chars) while i > 0: - name += new_name_chars[i % len(new_name_chars)] - i = i // len(new_name_chars) - if not is_reserved_keyword(name): + # Generate the remaining characters from 'self.additional_letter_chars' + name += self.additional_letter_chars[i % len(self.additional_letter_chars)] + i = i // len(self.additional_letter_chars) + self.name_index = self.name_index + 1 + return name + +# Exported variables only use upper case letters in their names. HLSL semantics are not case +# sensitive and may also assign special meaning to numbers. +upper_case_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" +upper_case_name_generator = NameGenerator(upper_case_chars, upper_case_chars + '_') + +# Don't begin new names with the the '_' character. Internal code can begin names with '_' without +# fear of renaming collisions. +lower_and_upper_chars = "abcdefghijklmnopqrstuvwxyz" + upper_case_chars +general_name_generator = NameGenerator(lower_and_upper_chars, "_0123456789" + lower_and_upper_chars) + +used_new_names = set() + +def generate_new_name(*, force_upper_case): + global upper_case_name_generator, general_name_generator, used_new_names; + name_generator = upper_case_name_generator if force_upper_case else general_name_generator + while True: + name = name_generator.next_name() + if not is_reserved_keyword(name) and not name in used_new_names: + used_new_names.add(name) return name # mapping from original identifiers to new names. @@ -323,7 +347,9 @@ for name,count in sorted(all_id_counts.items(), key=lambda x:x[1], reverse=True): new_name = (remove_leading_annotation(name) if args.human_readable or is_reserved_keyword(name) - else generate_new_name()) + # HLSL semantics are not case sensitive and can assign special meaning to + # numbers. Make all exported names upper case with no numbers. + else generate_new_name(force_upper_case=name[0] == '@')) new_names[name] = new_name # used to rewrite rgba and stpq swizzles to xyzw.
diff --git a/renderer/shaders/tessellate.glsl b/renderer/shaders/tessellate.glsl index 33a5447..566a6ff 100644 --- a/renderer/shaders/tessellate.glsl +++ b/renderer/shaders/tessellate.glsl
@@ -150,7 +150,8 @@ } v_contourIDWithFlags = contourIDWithFlags; - _pos.xy = coord * float2(2. / TESS_TEXTURE_WIDTH, uniforms.tessInverseViewportY) - 1.; + _pos.x = coord.x * (2. / TESS_TEXTURE_WIDTH) - 1.; + _pos.y = coord.y * uniforms.tessInverseViewportY - sign(uniforms.tessInverseViewportY); _pos.zw = float2(0, 1); VARYING_PACK(varyings, v_p0p1); @@ -158,7 +159,7 @@ VARYING_PACK(varyings, v_args); VARYING_PACK(varyings, v_joinArgs); VARYING_PACK(varyings, v_contourIDWithFlags); - EMIT_OFFSCREEN_VERTEX(varyings, _pos); + EMIT_VERTEX(varyings, _pos); } #endif