Get the Linux unit tests building and running in the GitHub workflow (#10761) a0b890f273

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

.rive_head

@@ -1 +1 @@
-a34d1ffe1563ced54b454058844dd49165eb9c42
+a0b890f2732314a2073a40139c5462dfc8b1b2dc

include/rive/math/simd.hpp

@@ -83,6 +83,22 @@ template <typename T> struct boolean_mask_type
 
 #define SIMD_NATIVE_GVEC 1
 
+// NOTE: Due to the build compiling with march=native, on systems with AVX
+//  or AVX512 support, it will output those vectors from these functions,
+//  but clang will kick out the following warning:
+//   AVX vector return of type 'gvec<signed char, 8 + 8 + 8 + 8>' (vector of
+//   32 'signed char' values) without 'avx' enabled changes the ABI
+//  In release builds, all of the places where these functions are being
+//   called should be getting inlined so there's not *technically* an ABI
+//   to be breaking, so disabling this should be fine.
+#define DISABLE_CLANG_SIMD_ABI_WARNING()                                       \
+    _Pragma("clang diagnostic ignored \"-Wpsabi\"")
+
+#define PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()                                  \
+    _Pragma("clang diagnostic push") DISABLE_CLANG_SIMD_ABI_WARNING()
+
+#define POP_DISABLE_CLANG_SIMD_ABI_WARNING() _Pragma("clang diagnostic pop")
+
 #else
 
 // gvec needs to be polyfilled with templates.
@@ -94,6 +110,11 @@ template <typename T> struct boolean_mask_type
 
 #define SIMD_NATIVE_GVEC 0
 
+// With non-native gvec these don't have to do anything
+#define DISABLE_CLANG_SIMD_ABI_WARNING()
+#define PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
+#define POP_DISABLE_CLANG_SIMD_ABI_WARNING()
+
 #endif
 
 namespace rive
@@ -585,6 +606,8 @@ SIMD_ALWAYS_INLINE std::
 }
 #endif
 
+PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
+
 template <typename T, int M, int N>
 SIMD_ALWAYS_INLINE gvec<T, M + N> join(gvec<T, M> a, gvec<T, N> b)
 {
@@ -696,6 +719,8 @@ SIMD_ALWAYS_INLINE
     return ret;
 }
 
+POP_DISABLE_CLANG_SIMD_ABI_WARNING()
+
 ////// Basic linear algebra //////
 
 template <typename T, int N>

include/utils/compile_time_string_hash.hpp

@@ -50,26 +50,26 @@ static constexpr unsigned int crc_table[256] = {
     0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8, 0x5d681b02, 0x2a6f2b94,
     0xb40bbe37, 0xc30c8ea1, 0x5a05df1b, 0x2d02ef8d};
 
-template <size_t IDX>
+template <std::size_t IDX>
 constexpr uint32_t combine_crc32(const char* str, uint32_t part)
 {
     return (part >> 8) ^ crc_table[(part ^ str[IDX]) & 0x000000FF];
 }
 
-template <size_t IDX> constexpr uint32_t crc32(const char* str)
+template <std::size_t IDX> constexpr uint32_t crc32(const char* str)
 {
     return combine_crc32<IDX>(str, crc32<IDX - 1>(str));
 }
 
 // This is the stop-recursion function
-template <> constexpr uint32_t crc32<size_t(-1)>(const char* str)
+template <> constexpr uint32_t crc32<std::size_t(-1)>(const char* str)
 {
     return 0xFFFFFFFF;
 }
 
 } // namespace detail
 
-template <size_t LEN> constexpr uint32_t ctcrc32(const char (&str)[LEN])
+template <std::size_t LEN> constexpr uint32_t ctcrc32(const char (&str)[LEN])
 {
     // LEN-1 is to remove the null terminator at the end of the string const.
     return detail::crc32<LEN - 1>(str) ^ 0xFFFFFFFF;

renderer/src/draw.cpp

@@ -1335,7 +1335,9 @@ void PathDraw::initForInteriorTriangulation(RenderContext* context,
                                             TriangulatorAxis triangulatorAxis)
 {
     RIVE_PROF_SCOPE()
+    PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
     assert(simd::all(m_resourceCounts.toVec() == 0)); // Only call init() once.
+    POP_DISABLE_CLANG_SIMD_ABI_WARNING()
     assert(!isStrokeOrFeather());
     assert(m_strokeRadius == 0);
 

renderer/src/intersection_board.cpp

@@ -140,6 +140,7 @@ int16x8 IntersectionTile::findMaxIntersectingGroupIndex(
     auto edges = m_edges.begin();
     auto groupIndices = m_groupIndices.begin();
     assert(m_edges.size() == m_groupIndices.size());
+    PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
     int8x32 complement = simd::join(r, b, _l, _t);
     for (; edges != m_edges.end(); ++edges, ++groupIndices)
     {
@@ -164,6 +165,8 @@ int16x8 IntersectionTile::findMaxIntersectingGroupIndex(
         runningMaxGroupIndices =
             simd::max(maskedGroupIndices, runningMaxGroupIndices);
     }
+    POP_DISABLE_CLANG_SIMD_ABI_WARNING()
+
 #else
     // MSVC doesn't get good codegen for the above loop. Provide direct SSE
     // intrinsics.

renderer/src/render_context.cpp

@@ -399,6 +399,7 @@ bool RenderContext::LogicalFlush::pushDraws(DrawUniquePtr draws[],
     RIVE_PROF_SCOPE()
     assert(!m_hasDoneLayout);
 
+    PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
     auto countsVector = m_resourceCounts.toVec();
     for (size_t i = 0; i < drawCount; ++i)
     {
@@ -407,6 +408,7 @@ bool RenderContext::LogicalFlush::pushDraws(DrawUniquePtr draws[],
                draws[i]->clipRectInverseMatrix() == nullptr);
         countsVector += draws[i]->resourceCounts().toVec();
     }
+    POP_DISABLE_CLANG_SIMD_ABI_WARNING()
     Draw::ResourceCounters countsWithNewBatch = countsVector;
 
     // Textures and buffers have hard size limits. If the new batch doesn't fit
@@ -705,6 +707,8 @@ void RenderContext::flush(const FlushResources& flushResources)
     assert(resourceRequirements.coverageBufferLength <=
            platformFeatures().maxCoverageBufferLength);
 
+    PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
+
     // Track m_maxRecentResourceRequirements so we can trim GPU allocations when
     // steady-state usage goes down.
     m_maxRecentResourceRequirements =
@@ -766,6 +770,8 @@ void RenderContext::flush(const FlushResources& flushResources)
         m_lastResourceTrimTimeInSeconds = flushTime;
     }
 
+    POP_DISABLE_CLANG_SIMD_ABI_WARNING()
+
     setResourceSizes(allocs);
 
     m_impl->prepareToFlush(flushResources.currentFrameNumber,
@@ -1039,8 +1045,11 @@ void RenderContext::LogicalFlush::layoutResources(
 
     m_flushDesc.externalCommandBuffer = flushResources.externalCommandBuffer;
 
+    PUSH_DISABLE_CLANG_SIMD_ABI_WARNING()
     *runningFrameResourceCounts =
         runningFrameResourceCounts->toVec() + m_resourceCounts.toVec();
+    POP_DISABLE_CLANG_SIMD_ABI_WARNING()
+
     runningFrameLayoutCounts->pathPaddingCount += m_pathPaddingCount;
     runningFrameLayoutCounts->paintPaddingCount += m_paintPaddingCount;
     runningFrameLayoutCounts->paintAuxPaddingCount += m_paintAuxPaddingCount;

src/animation/state_machine_instance.cpp

@@ -1315,7 +1315,7 @@ public:
         auto vmProp = dataContext->getViewModelProperty(path);
         if (vmProp != nullptr)
         {
-            m_viewModelInstanceValue = vmProp;
+            m_viewModelInstanceValue = rive::ref_rcp(vmProp);
             vmProp->addDependent(this);
         }
     }
@@ -1336,7 +1336,7 @@ public:
 private:
     StateMachineInstance* m_stateMachineInstance = nullptr;
     const StateMachineListener* m_listener = nullptr;
-    ViewModelInstanceValue* m_viewModelInstanceValue = nullptr;
+    rive::rcp<ViewModelInstanceValue> m_viewModelInstanceValue = nullptr;
 };
 
 } // namespace rive

tests/unit_tests/renderer/pls_render_context_test.cpp

@@ -6,6 +6,10 @@
 #include "common/render_context_null.hpp"
 #include <catch.hpp>
 
+// Rather than spot-disabling this warning at call sites, for test code
+//  we can just disable it for the whole file.
+DISABLE_CLANG_SIMD_ABI_WARNING()
+
 class RenderContextNULLTest : public RenderContextNULL
 {
 public:

tests/unit_tests/runtime/reader_test.cpp

@@ -42,8 +42,8 @@ TEST_CASE("string decoder", "[reader]")
 
     bytes_read = decode_string(12, str_bytes, str_bytes + 11, decoded_str);
     REQUIRE(bytes_read == 0);
-    delete str;
-    delete decoded_str;
+    free(str);
+    free(decoded_str);
 }
 
 TEST_CASE("float decoder", "[reader]")

tests/unit_tests/runtime/refcnt_test.cpp

@@ -78,6 +78,8 @@ TEST_CASE("rcp", "[basics]")
 
     struct A : public rive::RefCnt<A>
     {
+        virtual ~A() = default;
+
         int x = 17;
     };
     struct B : public A

tests/unit_tests/test.sh

@@ -100,29 +100,24 @@ RUNTIME=$PWD
 popd
 
 BUILD_RIVE_COMMANDS="$CONFIG --with_rive_tools --with_rive_audio=external --with_rive_scripting --no_ffp_contract $TOOLSET_ARG $EXTRA_CONFIG"
+$RUNTIME/build/build_rive.sh $BUILD_RIVE_COMMANDS
+
+rm -fR silvers/tarnished
+mkdir -p silvers/tarnished
 
 OUT_DIR="out/$CONFIG"
 
-if [[ $machine = "macosx" ]]; then
-  # Note that the build_rive.sh line is here (and in the windows block) and not outside of this (i.e. it doesn't always run)
-  #  because that is the way this script worked before it was converted to use build_rive.sh. This seems incorrect, but Linux
-  #  for instance currently does not build anything with this script (and the linux build fails on GitHub at PR time at the 
-  #  time of writing this, which is why for now it is this way instead).
-  build_rive.sh $BUILD_RIVE_COMMANDS
-  rm -fR silvers/tarnished
-  mkdir -p silvers/tarnished
-  $UTILITY $OUT_DIR/unit_tests "$MATCH"
-  if [[ $COVERAGE = "true" ]]; then
+# Actually run the unit tests
+$UTILITY $OUT_DIR/unit_tests "$MATCH"
+
+if [[ $COVERAGE = "true" ]]; then
+  if [[ $machine = "macosx" ]]; then
     xcrun llvm-profdata merge -sparse default.profraw -o default.profdata
     xcrun llvm-cov report $OUT_DIR/unit_tests -instr-profile=default.profdata
     # xcrun llvm-cov export out/debug/unit_tests -instr-profile=default.profdata -format=text >coverage.json
     xcrun llvm-cov export out/debug/unit_tests -instr-profile=default.profdata -format=lcov >coverage.txt
     sed -i '' -e 's?'$RUNTIME'?packages/runtime?g' coverage.txt
+  else
+    echo "'coverage' command line argument was specified but it only works on Mac so it was ignored"
   fi
-
-elif [[ $machine = "windows" ]]; then
-  build_rive.sh $BUILD_RIVE_COMMANDS
-  rm -fR silvers/tarnished
-  mkdir -p silvers/tarnished
-  $UTILITY $OUT_DIR/unit_tests "$MATCH"
-fi
+fi