Emit Apple .framework from the skiasharp_build GN target#254
Draft
mattleibow wants to merge 8 commits into
Draft
Emit Apple .framework from the skiasharp_build GN target#254mattleibow wants to merge 8 commits into
mattleibow wants to merge 8 commits into
Conversation
Compile the fork-owned C API shim (src/c/*.cpp) and its public headers
(include/c/*.h) in skiasharp_build("SkiaSharp") instead of injecting them
into upstream :core via gn/core.gni. libSkiaSharp deps directly on the
font-manager optional() targets and the modules the shim needs, so the
shim sees its public_defines (e.g. SK_FONTMGR_FONTCONFIG_AVAILABLE) and
GPU backend defines from its own dependency graph rather than riding on
:core's milestone-churning transitive deps.
Now that src/c no longer compiles in :core, remove the two fork
workarounds that only existed to feed it: the manual GPU defines block
(SK_VULKAN/SK_METAL/SK_DIRECT3D + use_skia_vulkan_headers) and the m148
fontconfig dep block (commit 029229d). Upstream :core's own sources
do not need these.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…SkiaSharp sources sk_path.cpp called the [[deprecated]] SkPathOps TightBounds() helper, which m148 marked deprecated. Inline its exact computeTightBounds() body instead so the C API shim no longer consumes a deprecated Skia API. The deprecation only failed Tizen's hand-written -Werror makefile build because the skiasharp_build GN template bypasses Skia's skia_target_default_configs, so //gn/skia:warnings (and thus -Wall/-Wextra) never reached our sources. Add :warnings for parity and a new :skiasharp_strict config (listed last) that forces -Werror=deprecated-declarations over the :warnings suppression, so any future deprecated-API use in src/c fails on every platform, not just Tizen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sk_path.cpp also used the // DEPRECATED bool/out-param overloads of Op, Simplify, AsWinding and SkOpBuilder::resolve. Unlike TightBounds these are only comment-deprecated (no [[deprecated]] attribute) so the compiler did not flag them, but upstream wants callers off them. Migrate all four to the std::optional<SkPath> returning forms, preserving the existing write-to-result + return-bool C API semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sk_linker.h, skottie_animation.h, skresources_resource_provider.h, and sksg_invalidation_controller.h were never in the C API public header list (carried over from the original gn/core.gni fork block). Now that :SkiaSharp owns the C API surface, complete the public header metadata so GN consumers and gn check see the full surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The is_mac ldflags emitted -Wl,macosx_version_min=$min_macos_version, which modern ld (Xcode 16+) treats as an input file path, breaking the dylib link. The deployment target is already conveyed via -target <cpu>-apple-macos$min, so the flag is both redundant and invalid. This link path was never exercised before because GN only built the static libskia.a on Apple (no link step); building libSkiaSharp.dylib directly via GN surfaces it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The is_ios and is_tvos device link paths used the -Wl,ios_version_min= and -Wl,tvos_version_min= forms, which modern ld interprets as input file paths (the same bug already fixed for is_mac). They are only exercised now that GN links the libSkiaSharp dylib directly instead of only producing the static libskia.a. Supply the deployment target to the linker via -target for both device and simulator instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When libSkiaSharp/libHarfBuzzSharp ship as Apple .framework bundles, the dynamic library install_name must be the framework-relative path rather than the bare @rpath/lib<name>.dylib default. Add skiasharp_apple_framework (+ _versioned) GN args that emit a -Wl,-install_name ldflag on both skiasharp_build targets, and reorder the solink toolchain rule so a target-supplied install_name wins by last-flag-wins. This replaces post-link install_name_tool surgery with a tool-driven link-time setting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move Apple framework assembly into the GN build itself. The skiasharp_build template (in gn/BUILDCONFIG.gn) now, when skiasharp_apple_framework is set, builds the dynamic library under an internal *_dylib target and runs a GN action that assembles a complete single-arch lib<Name>.framework in the out dir: bundle layout, the framework-relative install_name (already set at link time via the BUILD.gn ldflags), arm64e thinning, and a provenance-complete Info.plist (CFBundle* + DT*/BuildMachineOSBuild keys App Store/notarization asset validation expects). The packaging layer no longer owns the bundle shape, install_name or plist - it only lipos the per-arch frameworks together and code-signs. The assembler uses first-party Apple CLIs only (lipo/plutil/xcrun/xcodebuild/ sw_vers); the .py file is just the launcher GN's python3-pinned actions require and execs the .sh. The declare_args move from BUILD.gn to BUILDCONFIG.gn so the template can see them. macOS keeps shipping a plain dylib and every non-Apple platform is unaffected (args default false). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
dev/move-capi-to-libskiasharp
branch
from
4f9d7c6 to
db4f1e9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
SkiaSharp-fork build change: make the
skiasharp_buildGN targets produce the Apple.framework(and a correctly-linked dylib) directly, so SkiaSharp can buildlibSkiaSharp/libHarfBuzzSharpfor iOS/tvOS/Mac Catalyst from GN instead of hand-maintained Xcode projects. Companion to mono/SkiaSharp#4147.Commits
-Wl,macosx/ios/tvos_version_minldflags for the GN Apple dylib link — the deployment-target minimum is already supplied by the SDK/-target; the raw-Wl,..._version_minlinker flags are invalid under the current toolchain and broke the Apple dylib link.ldflagson bothskiasharp_buildtargets set@rpath/lib<N>.framework[/Versions/A]/lib<N>; a toolchain solink reorder ($rpathbefore{{ldflags}}) lets the target-supplied-install_namewin..frameworkbundle directly — whenskiasharp_apple_framework=true, the template links the dylib under an internal*_dylibtarget and runs a GNaction(gn/skiasharp/assemble_apple_framework.{py,sh}) that assembles a complete single-archlib<Name>.framework: bundle layout, arm64e thinning, and the provenance-completeInfo.plist(CFBundle* +DT*/BuildMachineOSBuildkeys App Store / notarization validation expects). First-party Apple CLIs only (lipo/plutil/xcrun/xcodebuild/sw_vers); the.pyis just the launcher GN's python3-pinned actions require. Thedeclare_argslive ingn/BUILDCONFIG.gnso the template sees them.Notes
skiasharp_apple_framework/_versioneddefault false — macOS and every non-Apple platform are unaffected (they keep the originaltarget()branch / plain dylib).dev/move-capi-to-libskiasharp; retarget toskiasharpafter that merges.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com