fix(vulkan): get MSAA working on systems without ClipDistance support (#10624) d1c02a29aa

Referencing ClipDistance (via gl_ClipDistance) in a SPIR-V shader, even if it is in a specialization branch that is not taken, is a validation error on systems that don't support ClipDistance. To get around this, there are now noclipdistance variations of the shaders that use it, and they (along with the MSAA shaders that never use ClipDistance) all define `DISABLE_CLIP_RECT_FOR_VULKAN_MSAA`, which ensures that it never tries to compile it in except where it matters.

Co-authored-by: Josh Jersild <joshua@rive.app>

.rive_head

@@ -1 +1 @@
-4d3770866ff47879152e42b967747750fef5c662
+d1c02a29aa51aa11b9759188780ecca84e823deb

build/rebuild_shaders.sh

@@ -1,19 +1,29 @@
+if [ -z "$RIVE_RENDERER_DIR" ]; then
+    echo "RIVE_RENDERER_DIR environment variable needs to be set for rebuild_shaders to run"
+    exit 1
+fi
+
 echo $RIVE_RENDERER_DIR
 pushd $RIVE_RENDERER_DIR > /dev/null
 echo Entered dir: $RIVE_RENDERER_DIR
 
 # Clean old shader files 
-dstShaderDir=$1
-echo Cleaning dir: $dstShaderDir
-rm -r $dstShaderDir
+dst_shader_dir=$1
+echo Cleaning dir: $dst_shader_dir
+rm -r $dst_shader_dir
+
+make_flags="--human-readable"
 
 # Find ply
 curr_dir=`pwd`
 found_ply_dir=`find ${curr_dir} -type d -wholename "*/dependencies/*/ply-*/src" -print`
-echo found ply: $found_ply_dir
+if [ -n "$found_ply_dir" ]; then
+    echo found ply: $found_ply_dir
+    make_flags="-p ${found_ply_dir} ${make_flags}"
+fi
 
 # Invoke make
-make -C ./src/shaders -j32 OUT=$dstShaderDir FLAGS="-p ${found_ply_dir} --human-readable" spirv-binary 
+make -C ./src/shaders -j32 OUT=$dst_shader_dir FLAGS="${make_flags}" spirv-binary 
 echo Exiting dir: $RIVE_RENDERER_DIR
 
 # Done 

renderer/path_fiddle/path_fiddle.cpp

@@ -8,6 +8,7 @@
 #include "rive/static_scene.hpp"
 #include "rive/profiler/profiler_macros.h"
 
+#include <filesystem>
 #include <fstream>
 #include <iterator>
 #include <vector>
@@ -847,7 +848,20 @@ void riveMainLoop()
         hotloadShaders = false;
 
 #ifndef RIVE_BUILD_FOR_IOS
-        std::system("sh rebuild_shaders.sh /tmp/rive");
+        // We want to build to /tmp/rive (or the correct equivalent)
+        std::filesystem::path tempRiveDir =
+            std::filesystem::temp_directory_path() / "rive";
+
+        // Get the u8 version of the path (this is especially necessary on
+        // windows where the native path character type is wchar_t, then
+        // reinterpret_cast the char8_t pointer to char so we can append it to
+        // our string.
+        const char* tempRiveDirPtr =
+            reinterpret_cast<const char*>(tempRiveDir.u8string().c_str());
+
+        std::string rebuildCommand =
+            std::string{"sh rebuild_shaders.sh "} + tempRiveDirPtr;
+        std::system(rebuildCommand.c_str());
 #endif
         fiddleContext->hotloadShaders();
     }

renderer/shader_hotload/shader_hotload.cpp

@@ -109,18 +109,21 @@ rive::Span<const uint32_t> loadNewShaderFileData()
         riveSpirvPath / "draw_clockwise_image_mesh.vert.spirv",
         riveSpirvPath / "draw_clockwise_image_mesh.frag.spirv",
 
-        riveSpirvPath / "draw_msaa_path.vert",
-        riveSpirvPath / "draw_msaa_path.frag",
-        riveSpirvPath / "draw_msaa_path.fixedcolor_frag",
-        riveSpirvPath / "draw_msaa_stencil.vert",
-        riveSpirvPath / "draw_msaa_stencil.frag",
-        riveSpirvPath / "draw_msaa_stencil.fixedcolor_frag",
-        riveSpirvPath / "draw_msaa_atlas_blit.vert",
-        riveSpirvPath / "draw_msaa_atlas_blit.frag",
-        riveSpirvPath / "draw_msaa_atlas_blit.fixedcolor_frag",
-        riveSpirvPath / "draw_msaa_image_mesh.vert",
-        riveSpirvPath / "draw_msaa_image_mesh.frag",
-        riveSpirvPath / "draw_msaa_image_mesh.fixedcolor_frag",
+        riveSpirvPath / "draw_msaa_path.vert.spirv",
+        riveSpirvPath / "draw_msaa_path.noclipdistance_vert.spirv",
+        riveSpirvPath / "draw_msaa_path.frag.spirv",
+        riveSpirvPath / "draw_msaa_path.fixedcolor_frag.spirv",
+        riveSpirvPath / "draw_msaa_stencil.vert.spirv",
+        riveSpirvPath / "draw_msaa_stencil.frag.spirv",
+        riveSpirvPath / "draw_msaa_stencil.fixedcolor_frag.spirv",
+        riveSpirvPath / "draw_msaa_atlas_blit.vert.spirv",
+        riveSpirvPath / "draw_msaa_atlas_blit.noclipdistance_vert.spirv",
+        riveSpirvPath / "draw_msaa_atlas_blit.frag.spirv",
+        riveSpirvPath / "draw_msaa_atlas_blit.fixedcolor_frag.spirv",
+        riveSpirvPath / "draw_msaa_image_mesh.vert.spirv",
+        riveSpirvPath / "draw_msaa_image_mesh.noclipdistance_vert.spirv",
+        riveSpirvPath / "draw_msaa_image_mesh.frag.spirv",
+        riveSpirvPath / "draw_msaa_image_mesh.fixedcolor_frag.spirv",
     };
     constexpr size_t numFiles = std::size(spirvFileNames);
 

renderer/src/shaders/Makefile

@@ -168,7 +168,10 @@ SPIRV_OUTPUTS_HEADERS := \
     $(OUT)/spirv/draw_interior_triangles.webgpu_frag.h \
     $(OUT)/spirv/draw_atlas_blit.webgpu_vert.h \
     $(OUT)/spirv/draw_atlas_blit.webgpu_frag.h \
-    $(OUT)/spirv/draw_image_mesh.webgpu_frag.h
+    $(OUT)/spirv/draw_image_mesh.webgpu_frag.h \
+	$(OUT)/spirv/draw_msaa_path.noclipdistance_vert.h \
+	$(OUT)/spirv/draw_msaa_image_mesh.noclipdistance_vert.h \
+	$(OUT)/spirv/draw_msaa_atlas_blit.noclipdistance_vert.h \
 
 SPIRV_OUTPUTS_BINARY := \
     $(addprefix $(OUT)/, $(patsubst %.main, %.vert.spirv, $(wildcard spirv/*.main))) \
@@ -184,8 +187,11 @@ SPIRV_OUTPUTS_BINARY := \
     $(OUT)/spirv/draw_msaa_atlas_blit.fixedcolor_frag.spirv \
     $(OUT)/spirv/draw_msaa_image_mesh.fixedcolor_frag.spirv \
     $(OUT)/spirv/draw_msaa_path.fixedcolor_frag.spirv \
-    $(OUT)/spirv/draw_msaa_stencil.fixedcolor_frag.spirv
-
+    $(OUT)/spirv/draw_msaa_stencil.fixedcolor_frag.spirv \
+	$(OUT)/spirv/draw_msaa_path.noclipdistance_vert.spirv \
+	$(OUT)/spirv/draw_msaa_atlas_blit.noclipdistance_vert.spirv \
+    $(OUT)/spirv/draw_msaa_image_mesh.noclipdistance_vert.spirv \
+    
 ## Compile *.main into vertex shaders. First rule generates the binary spirv, then the .h file after.
 $(OUT)/spirv/%.vert.spirv: spirv/%.main $(MINIFY_STAMP)
 	@mkdir -p $(OUT)/spirv
@@ -213,6 +219,15 @@ $(OUT)/spirv/%.fixedcolor_frag.h: spirv/%.main $(MINIFY_STAMP)
 	@mkdir -p $(OUT)/spirv
 	@glslangValidator -S frag -DTARGET_VULKAN -DFRAGMENT -DPLS_IMPL_SUBPASS_LOAD -DFIXED_FUNCTION_COLOR_OUTPUT -I$(OUT) -V --vn $(subst .main,_fixedcolor_frag,$(notdir $<)) -o $@ $<
 
+## Compile msaa vertex shaders again with DISABLE_CLIP_RECT_FOR_VULKAN_MSAA defined.
+$(OUT)/spirv/%.noclipdistance_vert.spirv: spirv/%.main $(MINIFY_STAMP)
+	@mkdir -p $(OUT)/spirv
+	@glslangValidator -S vert -DTARGET_VULKAN -DVERTEX -DDISABLE_CLIP_RECT_FOR_VULKAN_MSAA -I$(OUT) -V -o $@ $<
+
+$(OUT)/spirv/%.noclipdistance_vert.h: spirv/%.main $(MINIFY_STAMP)
+	@mkdir -p $(OUT)/spirv
+	@glslangValidator -S vert -DTARGET_VULKAN -DVERTEX -DDISABLE_CLIP_RECT_FOR_VULKAN_MSAA -I$(OUT) -V --vn $(subst .main,_noclipdistance_vert,$(notdir $<)) -o $@ $<
+
 ## Compile *.vert into vertex shaders.
 $(OUT)/spirv/%.vert.spirv: spirv/%.vert $(MINIFY_STAMP)
 	@mkdir -p $(OUT)/spirv

renderer/src/shaders/draw_image_mesh.glsl

@@ -17,7 +17,7 @@ NO_PERSPECTIVE VARYING(0, float2, v_texCoord);
 #ifdef @ENABLE_CLIPPING
 @OPTIONALLY_FLAT VARYING(1, half, v_clipID);
 #endif
-#ifdef @ENABLE_CLIP_RECT
+#if defined(@ENABLE_CLIP_RECT) && !defined(@RENDER_MODE_MSAA)
 NO_PERSPECTIVE VARYING(2, float4, v_clipRect);
 #endif
 VARYING_BLOCK_END
@@ -40,7 +40,7 @@ IMAGE_MESH_VERTEX_MAIN(@drawVertexMain,
 #ifdef @ENABLE_CLIPPING
     VARYING_INIT(v_clipID, half);
 #endif
-#ifdef @ENABLE_CLIP_RECT
+#if defined(@ENABLE_CLIP_RECT) && !defined(@RENDER_MODE_MSAA)
     VARYING_INIT(v_clipRect, float4);
 #endif
 

renderer/src/shaders/draw_path.glsl

@@ -45,7 +45,7 @@ NO_PERSPECTIVE VARYING(2, half2, v_coverages);
 #ifdef @ENABLE_CLIPPING
 @OPTIONALLY_FLAT VARYING(4, half2, v_clipIDs); // [clipID, outerClipID]
 #endif
-#ifdef @ENABLE_CLIP_RECT
+#if defined(@ENABLE_CLIP_RECT) && !defined(@RENDER_MODE_MSAA)
 NO_PERSPECTIVE VARYING(5, float4, v_clipRect);
 #endif
 #ifdef @ENABLE_ADVANCED_BLEND
@@ -81,7 +81,7 @@ VERTEX_MAIN(@drawVertexMain, Attrs, attrs, _vertexID, _instanceID)
 #ifdef @ENABLE_CLIPPING
     VARYING_INIT(v_clipIDs, half2);
 #endif
-#ifdef @ENABLE_CLIP_RECT
+#if defined(@ENABLE_CLIP_RECT) && !defined(@RENDER_MODE_MSAA)
     VARYING_INIT(v_clipRect, float4);
 #endif
 #ifdef @ENABLE_ADVANCED_BLEND

renderer/src/shaders/minify.py

@@ -35,17 +35,21 @@ parser.add_argument("-o", "--outdir", required=True,
                     help="OUTPUT directory to store the header files")
 parser.add_argument("-H", "--human-readable", action='store_true',
                     help="don't rename or strip out comments or whitespace")
-parser.add_argument("-p", "--ply-path", required=True, type=str, help="path to ply module")
+parser.add_argument("-p", "--ply-path", type=str, help="path to ply module")
 
 args = parser.parse_args()
 
-# Convert posix path to windows 
-convertedPath = args.ply_path
-if sys.platform.startswith('win32') and args.ply_path[:2] == '/c':
-    convertedPath = 'C:\\' + args.ply_path[2:]
-    print('Using ply path:' + convertedPath)
+if args.ply_path:
+    # --ply-path was specified, so add it to the sys path so we can locate the module.
+    # (if it was not specified we'll assume that it's already reachable via the path)
 
-sys.path.append(convertedPath)
+    # Convert posix path to windows 
+    convertedPath = args.ply_path
+    if sys.platform.startswith('win32') and args.ply_path[:2] == '/c':
+        convertedPath = 'C:\\' + args.ply_path[2:]
+        print('Using ply path:' + convertedPath)
+
+    sys.path.append(convertedPath)
 
 import ply.lex as lex
 

renderer/src/shaders/specialization.glsl

@@ -22,7 +22,17 @@ layout(constant_id = VULKAN_VENDOR_ID_SPECIALIZATION_IDX) const uint
     kVulkanVendorID = 0;
 
 #define @ENABLE_CLIPPING kEnableClipping
+
+// MSAA uses gl_ClipDistance when ENABLE_CLIP_RECT is set, but since Vulkan is
+// using specialization constants (as opposed to compile-time flags), it means
+// that the usage of them is in the compiled shader even if that codepath is
+// not going to be taken, which ends up as a validation failure on systems that
+// do not support that extension. In those cases, we can just not define
+// ENABLE_CLIP_RECT to avoid all of the gl_ClipDistance usages.
+#ifndef @DISABLE_CLIP_RECT_FOR_VULKAN_MSAA
 #define @ENABLE_CLIP_RECT kEnableClipRect
+#endif
+
 #define @ENABLE_ADVANCED_BLEND kEnableAdvancedBlend
 #define @ENABLE_FEATHER kEnableFeather
 #define @ENABLE_EVEN_ODD kEnableEvenOdd

renderer/src/shaders/spirv/draw_msaa_stencil.main

@@ -3,6 +3,11 @@
 #extension GL_EXT_samplerless_texture_functions : require
 #define OPTIONALLY_FLAT flat
 #define RENDER_MODE_MSAA
+
+// This shader doesn't need ENABLE_CLIP_RECT functionality. Disable it so that 
+// it does not fail to compile on systems without ClipDistance support.
+#define DISABLE_CLIP_RECT_FOR_VULKAN_MSAA
+
 #include "glsl.minified.glsl"
 #include "constants.minified.glsl"
 #include "specialization.minified.glsl"

renderer/src/vulkan/draw_shader_vulkan.cpp

@@ -203,7 +203,10 @@ DrawShaderVulkan::DrawShaderVulkan(Type type,
                 case DrawType::msaaMidpointFanStencilReset:
                 case DrawType::msaaMidpointFanPathsStencil:
                 case DrawType::msaaMidpointFanPathsCover:
-                    vertCode = spirv::draw_msaa_path_vert;
+                    vertCode =
+                        (shaderFeatures & ShaderFeatures::ENABLE_CLIP_RECT)
+                            ? spirv::draw_msaa_path_vert
+                            : spirv::draw_msaa_path_noclipdistance_vert;
                     fragCode = fixedFunctionColorOutput
                                    ? spirv::draw_msaa_path_fixedcolor_frag
                                    : spirv::draw_msaa_path_frag;
@@ -220,14 +223,20 @@ DrawShaderVulkan::DrawShaderVulkan(Type type,
                     break;
 
                 case DrawType::atlasBlit:
-                    vertCode = spirv::draw_msaa_atlas_blit_vert;
+                    vertCode =
+                        (shaderFeatures & ShaderFeatures::ENABLE_CLIP_RECT)
+                            ? spirv::draw_msaa_atlas_blit_vert
+                            : spirv::draw_msaa_atlas_blit_noclipdistance_vert;
                     fragCode = fixedFunctionColorOutput
                                    ? spirv::draw_msaa_atlas_blit_fixedcolor_frag
                                    : spirv::draw_msaa_atlas_blit_frag;
                     break;
 
                 case DrawType::imageMesh:
-                    vertCode = spirv::draw_msaa_image_mesh_vert;
+                    vertCode =
+                        (shaderFeatures & ShaderFeatures::ENABLE_CLIP_RECT)
+                            ? spirv::draw_msaa_image_mesh_vert
+                            : spirv::draw_msaa_image_mesh_noclipdistance_vert;
                     fragCode = fixedFunctionColorOutput
                                    ? spirv::draw_msaa_image_mesh_fixedcolor_frag
                                    : spirv::draw_msaa_image_mesh_frag;

renderer/src/vulkan/vulkan_shaders.cpp

@@ -63,15 +63,18 @@ namespace embedded
 #include "generated/shaders/spirv/draw_msaa_path.vert.h"
 #include "generated/shaders/spirv/draw_msaa_path.frag.h"
 #include "generated/shaders/spirv/draw_msaa_path.fixedcolor_frag.h"
+#include "generated/shaders/spirv/draw_msaa_path.noclipdistance_vert.h"
 #include "generated/shaders/spirv/draw_msaa_stencil.vert.h"
 #include "generated/shaders/spirv/draw_msaa_stencil.frag.h"
 #include "generated/shaders/spirv/draw_msaa_stencil.fixedcolor_frag.h"
 #include "generated/shaders/spirv/draw_msaa_atlas_blit.vert.h"
 #include "generated/shaders/spirv/draw_msaa_atlas_blit.frag.h"
 #include "generated/shaders/spirv/draw_msaa_atlas_blit.fixedcolor_frag.h"
+#include "generated/shaders/spirv/draw_msaa_atlas_blit.noclipdistance_vert.h"
 #include "generated/shaders/spirv/draw_msaa_image_mesh.vert.h"
 #include "generated/shaders/spirv/draw_msaa_image_mesh.frag.h"
 #include "generated/shaders/spirv/draw_msaa_image_mesh.fixedcolor_frag.h"
+#include "generated/shaders/spirv/draw_msaa_image_mesh.noclipdistance_vert.h"
 } // namespace embedded
 
 // Draw setup shaders.
@@ -171,6 +174,8 @@ rive::Span<const uint32_t> draw_clockwise_image_mesh_frag =
 // InterlockMode::msaa shaders.
 rive::Span<const uint32_t> draw_msaa_path_vert =
     rive::make_span(embedded::draw_msaa_path_vert);
+rive::Span<const uint32_t> draw_msaa_path_noclipdistance_vert =
+    rive::make_span(embedded::draw_msaa_path_noclipdistance_vert);
 rive::Span<const uint32_t> draw_msaa_path_frag =
     rive::make_span(embedded::draw_msaa_path_frag);
 rive::Span<const uint32_t> draw_msaa_path_fixedcolor_frag =
@@ -183,12 +188,16 @@ rive::Span<const uint32_t> draw_msaa_stencil_fixedcolor_frag =
     rive::make_span(embedded::draw_msaa_stencil_fixedcolor_frag);
 rive::Span<const uint32_t> draw_msaa_atlas_blit_vert =
     rive::make_span(embedded::draw_msaa_atlas_blit_vert);
+rive::Span<const uint32_t> draw_msaa_atlas_blit_noclipdistance_vert =
+    rive::make_span(embedded::draw_msaa_atlas_blit_noclipdistance_vert);
 rive::Span<const uint32_t> draw_msaa_atlas_blit_frag =
     rive::make_span(embedded::draw_msaa_atlas_blit_frag);
 rive::Span<const uint32_t> draw_msaa_atlas_blit_fixedcolor_frag =
     rive::make_span(embedded::draw_msaa_atlas_blit_fixedcolor_frag);
 rive::Span<const uint32_t> draw_msaa_image_mesh_vert =
     rive::make_span(embedded::draw_msaa_image_mesh_vert);
+rive::Span<const uint32_t> draw_msaa_image_mesh_noclipdistance_vert =
+    rive::make_span(embedded::draw_msaa_image_mesh_noclipdistance_vert);
 rive::Span<const uint32_t> draw_msaa_image_mesh_frag =
     rive::make_span(embedded::draw_msaa_image_mesh_frag);
 rive::Span<const uint32_t> draw_msaa_image_mesh_fixedcolor_frag =
@@ -253,15 +262,18 @@ void hotload_shaders(rive::Span<const uint32_t> spirvData)
     spirv::draw_clockwise_image_mesh_frag = readNextBytecodeSpan();
 
     spirv::draw_msaa_path_vert = readNextBytecodeSpan();
+    spirv::draw_msaa_path_noclipdistance_vert = readNextBytecodeSpan();
     spirv::draw_msaa_path_frag = readNextBytecodeSpan();
     spirv::draw_msaa_path_fixedcolor_frag = readNextBytecodeSpan();
     spirv::draw_msaa_stencil_vert = readNextBytecodeSpan();
     spirv::draw_msaa_stencil_frag = readNextBytecodeSpan();
     spirv::draw_msaa_stencil_fixedcolor_frag = readNextBytecodeSpan();
     spirv::draw_msaa_atlas_blit_vert = readNextBytecodeSpan();
+    spirv::draw_msaa_atlas_blit_noclipdistance_vert = readNextBytecodeSpan();
     spirv::draw_msaa_atlas_blit_frag = readNextBytecodeSpan();
     spirv::draw_msaa_atlas_blit_fixedcolor_frag = readNextBytecodeSpan();
     spirv::draw_msaa_image_mesh_vert = readNextBytecodeSpan();
+    spirv::draw_msaa_image_mesh_noclipdistance_vert = readNextBytecodeSpan();
     spirv::draw_msaa_image_mesh_frag = readNextBytecodeSpan();
     spirv::draw_msaa_image_mesh_fixedcolor_frag = readNextBytecodeSpan();
 }

renderer/src/vulkan/vulkan_shaders.hpp

@@ -63,15 +63,18 @@ extern rive::Span<const uint32_t> draw_clockwise_image_mesh_frag;
 
 // InterlockMode::msaa shaders.
 extern rive::Span<const uint32_t> draw_msaa_path_vert;
+extern rive::Span<const uint32_t> draw_msaa_path_noclipdistance_vert;
 extern rive::Span<const uint32_t> draw_msaa_path_frag;
 extern rive::Span<const uint32_t> draw_msaa_path_fixedcolor_frag;
 extern rive::Span<const uint32_t> draw_msaa_stencil_vert;
 extern rive::Span<const uint32_t> draw_msaa_stencil_frag;
 extern rive::Span<const uint32_t> draw_msaa_stencil_fixedcolor_frag;
 extern rive::Span<const uint32_t> draw_msaa_atlas_blit_vert;
+extern rive::Span<const uint32_t> draw_msaa_atlas_blit_noclipdistance_vert;
 extern rive::Span<const uint32_t> draw_msaa_atlas_blit_frag;
 extern rive::Span<const uint32_t> draw_msaa_atlas_blit_fixedcolor_frag;
 extern rive::Span<const uint32_t> draw_msaa_image_mesh_vert;
+extern rive::Span<const uint32_t> draw_msaa_image_mesh_noclipdistance_vert;
 extern rive::Span<const uint32_t> draw_msaa_image_mesh_frag;
 extern rive::Span<const uint32_t> draw_msaa_image_mesh_fixedcolor_frag;