Skip to content

Commit

Permalink
Final Skia Removal
Browse files Browse the repository at this point in the history
Removing the last vestiges of Skia:
- C++ unit tests (currently unused)
- Documentation in Rive.kt and Renderer.kt

Once this is merged, we can probably finally release Android.

Also, while I was in the neighborhood, I made the C++ unit tests (currently just unused scaffolding) at least runnable. After chatting with @umberto-sonnino, these were abandoned after he realized how tied they were to the JNI/JVM environment. This is a first step in exploring making them usable in the future. We would likely need to consider dependency injection and mocking of JNI methods and environment, which would be a larger effort.

Diffs=
edf4e518cd fix: Final Skia Removal (#9104)

Co-authored-by: Erik <[email protected]>
  • Loading branch information
ErikUggeldahl and ErikUggeldahl committed Feb 25, 2025
1 parent 0983be9 commit 9f4522f
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 28 deletions.
2 changes: 1 addition & 1 deletion .rive_head
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cfde2d5136e3fef2a27409c25fc002cbefe50398
edf4e518cda5038d4828e816665b1cd703c273d3
25 changes: 9 additions & 16 deletions kotlin/src/main/cpp/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ project(example LANGUAGES CXX VERSION 0.0.1)
include(FetchContent)

FetchContent_Declare(
Catch2
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
GIT_TAG v3.3.2
Catch2
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
GIT_TAG v3.3.2
)

FetchContent_MakeAvailable(Catch2)
Expand All @@ -17,19 +17,18 @@ set(CMAKE_CXX_FLAGS_DEBUG "-g")
set(CMAKE_CXX_FLAGS_RELEASE "-Oz")
set(CMAKE_VERBOSE_MAKEFILE ON)

set(SKIA_DIR_NAME "skia")
set(RIVE_ANDROID_CPP_DIR "${PROJECT_SOURCE_DIR}/../")
set(RIVE_RUNTIME_DIR "${PROJECT_SOURCE_DIR}/../../../runtime")
set(RIVE_RUNTIME_DIR "${PROJECT_SOURCE_DIR}/../../../../../../runtime")

add_library(rive-android-lib SHARED IMPORTED)
set_target_properties(rive-android-lib
PROPERTIES IMPORTED_LOCATION
${CMAKE_CURRENT_SOURCE_DIR}/output/ninja/arm64-v8a/librive-android.so
PROPERTIES IMPORTED_LOCATION
${CMAKE_CURRENT_SOURCE_DIR}/output/ninja/arm64-v8a/librive-android.so
)
add_executable(example_test_suite first_test.cpp)
target_link_libraries(example_test_suite PRIVATE
rive-android-lib
Catch2::Catch2WithMain
target_link_libraries(example_test_suite PRIVATE
rive-android-lib
Catch2::Catch2WithMain
)

list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
Expand All @@ -38,10 +37,4 @@ include_directories(
${RIVE_ANDROID_CPP_DIR}/include
${RIVE_RUNTIME_DIR}/include
${RIVE_RUNTIME_DIR}/renderer/library/include
${RIVE_RUNTIME_DIR}/skia/dependencies/${SKIA_DIR_NAME}/
${RIVE_RUNTIME_DIR}/skia/dependencies/${SKIA_DIR_NAME}/include/core
${RIVE_RUNTIME_DIR}/skia/dependencies/${SKIA_DIR_NAME}/include/effects
${RIVE_RUNTIME_DIR}/skia/dependencies/${SKIA_DIR_NAME}/include/gpu
${RIVE_RUNTIME_DIR}/skia/dependencies/${SKIA_DIR_NAME}/include/config
${RIVE_RUNTIME_DIR}/skia/renderer/include
)
6 changes: 3 additions & 3 deletions kotlin/src/main/cpp/test/first_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ TEST_CASE("Factorial of 0 is 1 (fail)", "[single-file]")

TEST_CASE("Factorials of 1 and higher are computed (pass)", "[single-file]")
{
rive::rcp<RefWorker> worker =
RefWorker::CurrentOrFallback(RendererType::Rive);
// rive::rcp<RefWorker> worker =
// RefWorker::CurrentOrFallback(RendererType::Rive);
printf("AM I ALIVE?!\n");
// worker->run([=](EGLThreadState* ts) { printf("I am alive!?\n"); });
REQUIRE(Factorial(1) == 1);
REQUIRE(Factorial(2) == 2);
REQUIRE(Factorial(3) == 6);
REQUIRE(Factorial(10) == 3628800);
printf("RELEASE...?\n");
worker.release();
// worker.release();
printf("RELEASED!\n");
}

Expand Down
12 changes: 6 additions & 6 deletions kotlin/src/main/cpp/test/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ mkdir -p build
pushd build

"${ANDROID_HOME}"/cmake/3.22.1/bin/cmake \
-H/Users/umbertosonnino/Projects/rive/packages/runtime_android/cpp \
-G Ninja \
-S "${PWD}"/../../ \
-B"${CMAKE_BUILD_TREE}" \
-DCMAKE_SYSTEM_NAME=Android \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DCMAKE_SYSTEM_VERSION=21 \
Expand All @@ -44,8 +46,6 @@ pushd build
-DCMAKE_LIBRARY_OUTPUT_DIRECTORY="${LIB_OUTPUT}" \
-DCMAKE_RUNTIME_OUTPUT_DIRECTORY="${LIB_OUTPUT}" \
-DCMAKE_BUILD_TYPE=Debug \
-B"${CMAKE_BUILD_TREE}" \
-GNinja \
-DCMAKE_VERBOSE_MAKEFILE=1 \
-DANDROID_ALLOW_UNDEFINED_SYMBOLS=ON \
-DANDROID_CPP_FEATURES="no-exceptions no-rtti" \
Expand All @@ -61,7 +61,9 @@ TEST_BUILD=build_catch_tests
mkdir -p ${TEST_BUILD}

"${ANDROID_HOME}"/cmake/3.22.1/bin/cmake \
-H. \
-G Ninja \
-S . \
-B"${TEST_BUILD}" \
-DCMAKE_SYSTEM_NAME=Android \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DCMAKE_SYSTEM_VERSION=21 \
Expand All @@ -75,8 +77,6 @@ mkdir -p ${TEST_BUILD}
-DCMAKE_LIBRARY_OUTPUT_DIRECTORY="${TEST_BUILD}" \
-DCMAKE_RUNTIME_OUTPUT_DIRECTORY="${TEST_BUILD}" \
-DCMAKE_BUILD_TYPE=Debug \
-B"${TEST_BUILD}" \
-GNinja \
-DCMAKE_VERBOSE_MAKEFILE=1 \
-DANDROID_ALLOW_UNDEFINED_SYMBOLS=ON \
-DANDROID_CPP_FEATURES="no-exceptions no-rtti" \
Expand Down
3 changes: 2 additions & 1 deletion kotlin/src/main/java/app/rive/runtime/kotlin/core/Rive.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ object Rive {
* instead.
*
* @param defaultRenderer The default renderer to use when initializing [File] or
* [RiveAnimationView]. Defaults to [RendererType.Skia].
* [app.rive.runtime.kotlin.RiveAnimationView][RiveAnimationView]. Defaults to
* [RendererType.Rive].
*/
fun init(context: Context, defaultRenderer: RendererType = RendererType.Rive) {
// NOTE: loadLibrary also allows us to specify a version, something we might want to take
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ abstract class Renderer(
/**
* Helper function to reassign the renderer type. This might be necessary if [constructor]
* couldn't build the renderer with [type] but had to fall back to a different value
* (e.g. the Rive Renderer isn't available on emulators and it defaults back to Skia).
* (e.g. the Rive Renderer isn't available on emulators and it defaults back to Canvas).
*/
private fun setRendererType(newType: Int) {
if (newType != type.value) {
Expand Down

0 comments on commit 9f4522f

Please sign in to comment.