Skip to content

feat: Add basic Skottie support#1987

Merged
mattleibow merged 11 commits into
mono:mainfrom
unoplatform:dev/jela/skottie
May 31, 2022
Merged

feat: Add basic Skottie support#1987
mattleibow merged 11 commits into
mono:mainfrom
unoplatform:dev/jela/skottie

Conversation

@jeromelaban

@jeromelaban jeromelaban commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Description of Change

Adds basic support for Skottie (dependent on mono/skia#84)

Related to issue #1580 #661

Size changes:

  • Wasm (2.0.23/libSkiaSharp.a):
    • 2.88.0-preview.232: 13379 KB
    • This PR: 14172 KB (+793 KB)
  • Android arm64 (libSkiaSharp.so):
    • 2.88.0-preview.232: 7303 KB
    • This PR: 7571 KB (+268KB)
  • iOS (libSkiaSharp.framework):
    • 2.88.0-preview.232: 23148 KB
    • This PR: 24211 KB (+1063KB)

Bugs Fixed

Fixes #661

API Changes
Added:

  • SkiaSharp.Skottie.Animation
  • SkiaSharp.Skottie.InvlidationController

Required skia PR

mono/skia#84

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation

@mattleibow

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jeromelaban
jeromelaban marked this pull request as ready for review April 4, 2022 12:32
@jeromelaban

Copy link
Copy Markdown
Contributor Author

Uno 4.1.9 is binary compatible with .NET 6 Mobile RC1, so given that RC1 is used by the repository, the build should be now fine.

@mattleibow mattleibow left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mostly hidden interfaces and naming in a pool of bad names...

Otherwise, this is a nice addition!

Comment thread binding/Binding/Skottie/Animation.cs Outdated
Comment thread binding/Binding/Skottie/Animation.cs Outdated
}

// SK_C_API skottie_animation_t* skottie_animation_make_from_string(const char* data, size_t length);
public static Animation Make (string data)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To .NET-ify, this should be Parse and to fully make it nice, should we add a TryParse?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've changed all of them to TryCreate or TryParse. The native implementation returns null when the parsing failed, so the only .NET semantic that makes sense here would be a TryParse, or Parse with a nullable return (which is not current available in this solution).

Comment thread binding/Binding/Skottie/Animation.cs Outdated
Comment thread binding/Binding/Skottie/Animation.cs Outdated
Comment thread binding/Binding/Skottie/SkiaApi.cs Outdated
Comment thread binding/Binding/Skottie/Animation.cs Outdated
Comment thread binding/Binding/Skottie/InvalidationController.cs Outdated
Comment thread binding/Binding/Skottie/Animation.cs Outdated
Comment thread tests/Tests/Skottie/AnimationTest.cs Outdated
Comment thread tests/Tests/Skottie/AnimationTest.cs
Comment thread binding/Binding/Skottie/Animation.cs Outdated
@mattleibow mattleibow added this to the v2.88.1 milestone May 28, 2022
@jeromelaban
jeromelaban marked this pull request as draft May 29, 2022 00:24
@jeromelaban
jeromelaban marked this pull request as ready for review May 29, 2022 13:08
@jeromelaban
jeromelaban marked this pull request as draft May 30, 2022 13:04
@jeromelaban

Copy link
Copy Markdown
Contributor Author

I added 21e285d to validate that the build passes fully, will remove once it does.

@jeromelaban
jeromelaban force-pushed the dev/jela/skottie branch 2 times, most recently from 305d14e to b6d7822 Compare May 30, 2022 15:25
@jeromelaban
jeromelaban marked this pull request as ready for review May 30, 2022 15:29
mattleibow
mattleibow previously approved these changes May 30, 2022
mattleibow
mattleibow previously approved these changes May 31, 2022
@jeromelaban
jeromelaban force-pushed the dev/jela/skottie branch 2 times, most recently from 7ccaa61 to 1178a72 Compare May 31, 2022 13:14
@mattleibow mattleibow self-assigned this May 31, 2022
@mattleibow mattleibow added type/enhancement area/SkiaSharp Issues that relate to the C# binding of SkiaSharp. area/libSkiaSharp.native labels May 31, 2022
@amirvenus

Copy link
Copy Markdown

How would we specify the Assets folder for png files?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/libSkiaSharp.native area/SkiaSharp Issues that relate to the C# binding of SkiaSharp. type/enhancement

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Feature request: Support Skottie

3 participants