Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ dev/analysis_report
build/bin
dev/test/build/bin
rivinfo/build/macosx
renderer/out

# Skia dependencies
skia/dependencies/skia_recorder
Expand Down
45 changes: 44 additions & 1 deletion build/rive_build_config.lua
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,12 @@ if _OPTIONS['with_optick'] then
RIVE_OPTICK_VERSION = '1.4.0.0'
end

-- Optional pthread enabling (used e.g. for Emscripten wagyu builds)
newoption({
trigger = 'with_pthread',
description = 'Enable pthread compile/link flags (-pthread)',
})

location(RIVE_BUILD_OUT)
targetdir(RIVE_BUILD_OUT)
objdir(RIVE_BUILD_OUT .. '/obj')
Expand Down Expand Up @@ -422,7 +428,7 @@ if _OPTIONS['for_android'] then

-- Detect the NDK.
local EXPECTED_NDK_VERSION = 'r27c'
local NDK_LONG_VERSION_STRING = "27.2.12479018"
local NDK_LONG_VERSION_STRING = "25.2.9519653"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for calling attention to this! @ErikUggeldahl can probably suggest how we should handle it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, looks like this wasn't fully handled. Right now it's 27 to match with Rive Android's expected build. By changing this, we will likely clash. I could see adding a parameter to change it if necessary, but just changing it like this is likely to cause issues.

Additionally:

  • This no longer matches the EXPECTED variable above.
  • This version does not support 16KB page sizes by default. By dropping <26, you would need to ensure the advised build flags are enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this entire PR is highlighting customer hot spots. So, what we likely need is a good way to specify the NDK/SDK version. I'm not sure what is the best way to do it, but basically something like this is a common approach:

export ANDROID_SDK_ROOT=/path/to/android/sdk
export ANDROID_NDK_ROOT=/path/to/android/ndk
export ANDROID_ABI=arm64-v8a
export ANDROID_PLATFORM=android-21
export ANDROID_TOOLCHAIN=clang
export ANDROID_STL=c++_shared

And then you will of course have your own defaults.

if _OPTIONS['for_unreal'] then
EXPECTED_NDK_VERSION = '25.1.8937393'
NDK_LONG_VERSION_STRING = '25.1.8937393'
Expand Down Expand Up @@ -559,6 +565,43 @@ end

filter({})

-- Always build position independent code on Linux
-- Ensures static libs can link into shared objects without relocation errors.
filter({ 'system:linux' })
do
pic('on')
buildoptions({ '-fPIC' })
linkoptions({ '-fPIC' })
end
Comment on lines +568 to +575
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using the "--with-pic" option, but it seems it didn't apply to everything required 🤷


-- Cross-compilation helpers for Linux. When targeting a Linux architecture that does not match
-- the host, add an explicit target triple so clang uses the correct backend. Optionally honor a
-- provided --sysroot for both compilation and linking.
if os.target() == 'linux' then
local host_arch = os.outputof('uname -m') or ''

-- Apply target triple if host arch doesn't match requested arch
if _OPTIONS['arch'] == 'x64' and host_arch ~= 'x86_64' then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for handling these. Can we add x86, just for completeness?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try adding it.

buildoptions({ '--target=x86_64-linux-gnu' })
linkoptions({ '--target=x86_64-linux-gnu' })
elseif _OPTIONS['arch'] == 'x86' and host_arch ~= 'i686' and host_arch ~= 'i386' then
buildoptions({ '--target=i686-linux-gnu' })
linkoptions({ '--target=i686-linux-gnu' })
elseif _OPTIONS['arch'] == 'arm64' and host_arch ~= 'aarch64' then
buildoptions({ '--target=aarch64-linux-gnu' })
linkoptions({ '--target=aarch64-linux-gnu' })
elseif _OPTIONS['arch'] == 'arm' and not host_arch:match('arm') then
buildoptions({ '--target=arm-linux-gnueabihf' })
linkoptions({ '--target=arm-linux-gnueabihf' })
end
end

-- Apply pthread flags when requested
if _OPTIONS['with_pthread'] then
buildoptions({ '-pthread' })
linkoptions({ '-pthread' })
end

filter('system:macosx')
do
defines({ 'RIVE_MACOSX' })
Expand Down
6 changes: 3 additions & 3 deletions renderer/premake5.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ dofile(RIVE_RUNTIME_DIR .. '/decoders/premake5_v2.lua')
dofile(RIVE_RUNTIME_DIR .. '/dependencies/premake5_glfw_v2.lua')

newoption({ trigger = 'with-skia', description = 'use skia' })
if _OPTIONS['with-skia'] then
if _OPTIONS['with-skia'] and not _OPTIONS['with-libs-only'] then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach may be obsolete with the workaround Chris suggested.

dofile(RIVE_RUNTIME_DIR .. '/skia/renderer/premake5_v2.lua')
end

if not _OPTIONS['with-webgpu'] then
if not _OPTIONS['with-webgpu'] and not _OPTIONS['with-libs-only'] then
project('path_fiddle')
do
dependson('rive')
Expand Down Expand Up @@ -179,7 +179,7 @@ if not _OPTIONS['with-webgpu'] then
end
end

if _OPTIONS['with-webgpu'] or _OPTIONS['with-dawn'] then
if (_OPTIONS['with-webgpu'] or _OPTIONS['with-dawn']) and not _OPTIONS['with-libs-only'] then
project('webgpu_player')
do
kind('ConsoleApp')
Expand Down
5 changes: 5 additions & 0 deletions renderer/premake5_pls_renderer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,11 @@ do
defines({ 'RIVE_WEBGPU=' .. _OPTIONS['webgpu-version'] })
end

newoption({
trigger = 'with-libs-only',
description = 'only builds the libraries',
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be necessary. If you want to build just a library, you can already say:

build_rive.sh -- rive_pls_renderer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I just verified that it is indeed possible to do, there are still major flaws with that approach:

  1. Documentation. There is no clear documentation as to which libraries are needed in order to link an executable.
  2. Robustness. There have been numerous updates (breaking changes) where the list (and names) of these libraries change.

So, in all fairness, I think the notion of a "build libraries only" target is still very much needed here.


filter({})

newoption({
Expand Down