Samsung / rlottie

A platform independent standalone library that plays Lottie Animation.
Other
1.18k stars 225 forks source link

Linker error on iOS and Apple Silicon: Undefined symbol _pixman_composite_over_n_8888_asm_neon #496

Open Ravbug opened 3 years ago

Ravbug commented 3 years ago

This is a more detailed error report for #488.

When compiling for iOS or Apple silicon, two symbols are not defined:

Undefined symbols for architecture arm64:
  "_pixman_composite_over_n_8888_asm_neon", referenced from:
      color_SourceOver(unsigned int*, int, unsigned int, unsigned int) in vdrawhelper_neon.o
  "_pixman_composite_src_n_8888_asm_neon", referenced from:
      memfill32(unsigned int*, unsigned int, int) in vdrawhelper_neon.o
ld: symbol(s) not found for architecture arm64

The file that uses these functions is vdrawhelper_neon.cpp.

Steps to Reproduce:

  1. Download rlottie source code on a Mac with Xcode installed.
  2. Edit the root CMakeLists.txt to include set(CMAKE_OSX_ARCHITECTURES "x86_64;arm64" CACHE INTERNAL "") below the project line, this will enable building Apple Silicon:
    
    cmake_minimum_required( VERSION 3.3 )

declare project

project( rlottie VERSION 0.2 LANGUAGES C CXX ASM) set(CMAKE_OSX_ARCHITECTURES "x86_64;arm64" CACHE INTERNAL "") # add this line

...


3. In the rlottie root directory, execute `mkdir -p build; cd build; cmake -G "Xcode" ..`
4. Open the Xcode project and try to build the rlottie library target.

**Other Notes**:
In the file `pixman-arm-neon-asm.S` (the file that contains the definitions for `pixman_composite_over_n_8888_asm_neon` and `pixman_composite_src_n_8888_asm_neon`), comments mention that there exists a C function `fast_composite_over_8888_0565` in `pixman-fast-path.c` which implements an identical behavior to the arm assembly functions. Perhaps use that function instead in `vdrawhelper_neon.cpp`, when the target does not support the assembly implementation? 
Siloow commented 3 years ago

Also running into this problem at the moment.

RocketTree commented 2 years ago

I am also hitting this issue. Has anyone come up with a solution? Short term fix I am thinking undef __ARM_NEON__ ?

RocketTree commented 2 years ago

oh, this only happens when building lottie2gif. The core rlottie project does not have this issue. I have just changed my build script to call make rlottie rather than make and it builds fine. EDIT: Well I should have expected to fall back to this issue. Once I got my unrelated xcode compilation issues solved I am back to this linking error.

RocketTree commented 2 years ago

I have worked around this locally, I changed the call in memfill32 to memset, and took the generic c implementation off source_over from vdrawhelper_common.cpp. I know this isnt the cleanest fix, but now I have rlottie running on iOS. Here is the patch of my changes for anyone interested.


 src/vector/vdrawhelper_neon.cpp | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/vector/vdrawhelper_neon.cpp b/src/vector/vdrawhelper_neon.cpp
index 681eabb..1c1eed6 100644
--- a/src/vector/vdrawhelper_neon.cpp
+++ b/src/vector/vdrawhelper_neon.cpp
@@ -2,28 +2,20 @@

 #include "vdrawhelper.h"

-extern "C" void pixman_composite_src_n_8888_asm_neon(int32_t w, int32_t h,
-                                                     uint32_t *dst,
-                                                     int32_t   dst_stride,
-                                                     uint32_t  src);
-
-extern "C" void pixman_composite_over_n_8888_asm_neon(int32_t w, int32_t h,
-                                                      uint32_t *dst,
-                                                      int32_t   dst_stride,
-                                                      uint32_t  src);
-
 void memfill32(uint32_t *dest, uint32_t value, int length)
 {
-    pixman_composite_src_n_8888_asm_neon(length, 1, dest, length, value);
+    memset(dest, value, length);
 }

 static void color_SourceOver(uint32_t *dest, int length,
                                       uint32_t color,
-                                     uint32_t const_alpha)
+                                     uint32_t alpha)
 {
-    if (const_alpha != 255) color = BYTE_MUL(color, const_alpha);
+    int ialpha, i;

-    pixman_composite_over_n_8888_asm_neon(length, 1, dest, length, color);
+    if (alpha != 255) color = BYTE_MUL(color, alpha);
+    ialpha = 255 - vAlpha(color);
+    for (i = 0; i < length; ++i) dest[i] = color + BYTE_MUL(dest[i], ialpha);
 }

 void RenderFuncTable::neon()
--
leonstyhre commented 2 years ago

I also have this problem when building rlottie on the Rasberry Pi using GCC. And I can confirm that it does not only happen when building lottie2gif as I only build the main library and still encounter the problem.

Applying the patch above solves the problems, so thanks for that! But it's not nice having to apply a custom patch to my repository so my application can be built successfully. Would it therefore be possible to apply this patch to the rlottie master branch or fix the problem in some other way?

Thanks! :)

framinlab commented 2 years ago

Did the above patch which helped with the build, but when time came to do the install, it tried to move all the binary artifacts into /usr/lib.

This is no longer allowed under Apple's SIP security guidelines. You can manually disable SIP but this is bad security practice and may not even be allowed for certain environments.

The solution is to edit cmake_install.cmake and lottie_workdir/build/src/vector/stb/cmake_install.cmake and change all the occurrences of /usr/lib to /usr/local/lib (which is writeable). You may also need to manually create a /usr/local/lib/cmake directory.

Then when time comes to use the libraries, you have to make sure the include and lib paths are properly set up to point at /usr/local/lib.

HTH.

XieJiSS commented 2 years ago

Wrapping up solutions provided above, with some modification:

  1. Changed install directory from /usr/lib to /usr/local/lib
  2. Fixed build error on newer gcc due to -Werror=sign-compare
  3. Fixed undefined symbol _pixman_composite_over_n_8888_asm_neon
diff --git a/CMakeLists.txt b/CMakeLists.txt
index e0d4e0b..f0a84e5 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -87,7 +87,7 @@ endif()

 if (NOT LIB_INSTALL_DIR)
-    set (LIB_INSTALL_DIR "/usr/lib")
+    set (LIB_INSTALL_DIR "/usr/local/lib")
 endif (NOT LIB_INSTALL_DIR)

 #declare source and include files
diff --git a/src/lottie/lottiemodel.cpp b/src/lottie/lottiemodel.cpp
index bf0e357..e96a043 100644
--- a/src/lottie/lottiemodel.cpp
+++ b/src/lottie/lottiemodel.cpp
@@ -188,11 +188,11 @@ void LOTGradient::populate(VGradientStops &stops, int frameNo)
         return;
     }
     int            colorPoints = mColorPoints;
-    if (colorPoints < 0 || colorPoints > size / 4) {  // for legacy bodymovin (ref: lottie-android)
+    if (colorPoints < 0 || (long unsigned int)(colorPoints) > size / 4) {  // for legacy bodymovin (ref: lottie-android)
         colorPoints = int(size / 4);
     }
     auto    opacityArraySize = size - colorPoints * 4;
-    if ((opacityArraySize % 2 != 0) || (colorPoints > opacityArraySize / 2 && opacityArraySize < 4)) {
+    if ((opacityArraySize % 2 != 0) || ((long unsigned int)(colorPoints) > opacityArraySize / 2 && opacityArraySize < 4)) {
         opacityArraySize = 0;
     }
     float *opacityPtr = ptr + (colorPoints * 4);
diff --git a/src/vector/vdrawhelper_neon.cpp b/src/vector/vdrawhelper_neon.cpp
index 99fd34f..a4a791a 100644
--- a/src/vector/vdrawhelper_neon.cpp
+++ b/src/vector/vdrawhelper_neon.cpp
@@ -2,27 +2,18 @@

 #include "vdrawhelper.h"

-extern "C" void pixman_composite_src_n_8888_asm_neon(int32_t w, int32_t h,
-                                                     uint32_t *dst,
-                                                     int32_t   dst_stride,
-                                                     uint32_t  src);
-
-extern "C" void pixman_composite_over_n_8888_asm_neon(int32_t w, int32_t h,
-                                                      uint32_t *dst,
-                                                      int32_t   dst_stride,
-                                                      uint32_t  src);
-
 void memfill32(uint32_t *dest, uint32_t value, int length)
 {
-    pixman_composite_src_n_8888_asm_neon(length, 1, dest, length, value);
+    memset(dest, value, length);
 }

 void Vcomp_func_solid_SourceOver_neon(uint32_t *dest, int length,
                                       uint32_t color,
                                      uint32_t const_alpha)
 {
+    int ialpha, i;
     if (const_alpha != 255) color = BYTE_MUL(color, const_alpha);
-
-    pixman_composite_over_n_8888_asm_neon(length, 1, dest, length, color);
+    ialpha = 255 - vAlpha(color);
+    for (i = 0; i < length; ++i) dest[i] = color + BYTE_MUL(dest[i], ialpha);
 }
 #endif

We may also use #if !defined(__APPLE__) to wrap those extern "C" void pixman_composite_src_n_8888_asm_neon stuff, so that other arm platforms can still benefit from the speedup.

onehundredfeet commented 1 year ago

Just curious when this will get rolled into the main line. I don't see a PR open.

exzos28 commented 9 months ago

😬

gilzoide commented 8 months ago

I'd say the correct way of fixing this is not removing the usage of neon instructions from vdrawhelper_neon.cpp, but rather not including/using neon in other places like vdrawhelper.cpp and vdrawhelper_common.cpp. I've opened the PR 556 mentioned above that only includes neon instructions on 32-bit arm architectures, fixing the builds for arm64 like Apple Silicon.