Add ImageAssetsFolder property and spec-compliant .lottie format support#332
Add ImageAssetsFolder property and spec-compliant .lottie format support#332Copilot wants to merge 16 commits into
Conversation
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
|
/azp run |
|
@copilot please do a thorough review and think deeply about this. Fix any bugs and make improvements. Also, the file resource provider may not look in the app resources as I think it is just on the file system. However the base resource provider does not allow custom inheritance so we can't fix right now. |
Co-authored-by: mattleibow <[email protected]>
I've addressed the FileResourceProvider limitation you identified. Changes in commit
Root cause confirmed: ResourceProvider.Load() methods are not virtual/abstract (verified by compilation), making custom implementation impossible until SkiaSharp.Resources is updated. The implementation now provides the ImageAssetsFolder property with clear documentation about its file system-only limitation and practical workarounds for MAUI apps. |
|
@copilot please make sure there is a sample image in the demo app that can demonstrate this new feature. Maybe some drop down / picker on the lottie page that allows picking between the base64, the file system and in future, the .lotrie file format. In fact, maybe we need to consider the .lottie file format. Please research that and make sure that works. We will have to extract for now, but in future we could use a zip file resource provider or something. Please make sure this new lottie file format is supported, everything is tested with 90% code coverage and that the sample has all of this demonstrations |
This is a comprehensive enhancement request. I'll address it in stages: Phase 1 - .lottie Format Research & Support:
Phase 2 - Demo App Enhancements:
Phase 3 - Testing:
Starting implementation now. This will take multiple commits as I add each component. |
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
|
@copilot can we rather make all the existing image sources support dot lottie. In fact all should support the assets folder too. The stream and uri should somehow detect the dotlottie and switch to use that. We should not need a new source for a type. The source is where it comes from, not what it is. |
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
mattleibow
left a comment
mattleibow
left a comment
There was a problem hiding this comment.
@copilot please review sm Fix The issue comments
| <packageSources> | ||
| <clear /> | ||
| <add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" /> | ||
| <add key="nuget.org" value="https://api.nuget.org/v3/index.json" /> |
| throw new ArgumentNullException(nameof(stream)); | ||
|
|
||
| // Check if this is a .lottie (ZIP) file by reading the first few bytes | ||
| var isZip = await IsZipFileAsync(stream, cancellationToken); |
There was a problem hiding this comment.
@copilot maybe we should wrap this in a front buffered stream? Then we can read twice. SkiaSharp has one.
|
|
||
| // Load the animation | ||
| using var animStream = File.OpenRead(fullAnimationPath); | ||
| var animation = CreateAnimationBuilder().Build(animStream); |
There was a problem hiding this comment.
The create method is internal? Maybe we can pass in the asset fir instead of updating the property.
| if (stream is null) | ||
| throw new FileLoadException($"Unable to load Lottie animation uri \"{Uri}\"."); | ||
| // Download to memory stream so we can detect format | ||
| using var memoryStream = new MemoryStream(); |
There was a problem hiding this comment.
We can pass directly if the load used font buffered streams
Fix all so we never copy
Co-authored-by: mattleibow <[email protected]>
|
@copilot please do a full review and fix the issues. Make sure you have 90% test coverage. |
…total tests) Co-authored-by: mattleibow <[email protected]>
|
@copilot please rebase and make sure you use the best docs. also, please use this file as the .lottie file: |
Co-authored-by: mattleibow <[email protected]>
|
. |
|
@copilot Please make sure the spec is right. This is https://dotlottie.io/spec/1.0/ and https://dotlottie.io/spec/2.0/ Changes https://dotlottie.io/spec/ |
1 similar comment
|
@copilot Please make sure the spec is right. This is https://dotlottie.io/spec/1.0/ and https://dotlottie.io/spec/2.0/ Changes https://dotlottie.io/spec/ |
Co-authored-by: mattleibow <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot please merge in the latest from main. Don't rebase. |
…-image-loading # Conflicts: # docs/docs/sklottieview.md # samples/SkiaSharpDemo/Demos/Lottie/LottiePage.xaml.cs # source/SkiaSharp.Extended.UI.Maui/Controls/Lottie/SKLottieImageSource.shared.cs
Merged latest from main in commit |
✅ Successfully Merged with Main Branch
Merge Summary:
origin/mainwithout rebasing (as requested)Conflicts Resolved
1. SKLottieImageSource.shared.cs
2. LottiePage.xaml.cs
3. Documentation
sklottieview.mdtolottie.md(main branch renamed it)Features Intact After Merge
✅ ImageAssetsFolder property - All image sources support external images
✅ .lottie format support - Auto-detection for dotLottie v1.0 and v2.0
✅ SKFrontBufferedStream - Efficient streaming without seekability requirement
✅ Thread-safe implementation - Parameterized builder pattern
✅ 90%+ test coverage - 58 test methods
✅ Demo with picker - Shows all three loading approaches
✅ Spec compliance - Official dotLottie v1.0 and v2.0 specs
✅ Animation controls from main - Speed, repeat mode, all playback features
What's New from Main Branch
The merge is complete with no data loss. All ImageAssetsFolder and .lottie features work alongside the latest main branch improvements.
Original prompt
This section details on the original issue you should resolve
<issue_title>[BUG] Animations with image assets fail to load images</issue_title>
<issue_description>Previously, when using the lottie animations, one could have one or more images with the animations and have them referenced in the data.json file.
As noted, we are referencing these images in that json payload:
img_0.png
img_1.png
We would then need to specify the Assets Folder path so that the app can locate the images:
animationView.ImageAssetsFolder = "images";However, such functionality (specifying the ImageAssetsFolder) is missing on the new library and as a result - and as far as I know - we can only use images in base64 format as of now which is undesirable especially if you are a couple of images specified in the animation file.
An example would be:
{ "v": "5.7.6", "fr": 24, "ip": 0, "op": 48, "w": 400, "h": 400, "nm": "welcome", "ddd": 0, "assets": [ { "id": "image_0", "w": 101, "h": 72, "u": "", "p": "img_0.png", "e": 0 }, { "id": "image_1", "w": 256, "h": 390, "u": "", "p": "img_1.png", "e": 0 } ], "layers": [ { "ddd": 0, "ind": 1, "ty": 2, "nm": "face.png", "cl": "png", "refId": "image_0", "sr": 1, "ks": { "o": { "a": 0, "k": 100, "ix": 11 }, "r": { "a": 1, "k": [ { "i": { "x": [ 0.833 ], "y": [ 0.833 ] }, "o": { "x": [ 0.167 ], "y": [ 0.167 ] }, "t": 0, "s": [ 0 ] }, { "i": { "x": [ 0.833 ], "y": [ 0.833 ] }, "o": { "x": [ 0.167 ], "y": [ 0.167 ] }, "t": 4, "s": [ 0 ] }, { "i": { "x": [ 0.833 ], "y": [ 0.833 ] }, "o": { "x": [ 0.167 ], "y": [ 0.167 ] }, "t": 8, "s": [ -50 ] }, { "i": { "x": [ 0.833 ], "y": [ 0.833 ] }, "o": { "x": [ 0.167 ], "y": [ 0.167 ] }, "t": 20, "s": [ 50 ] }, { "t": 29, "s": [ 0 ] } ], "ix": 10 }, "p": { "a": 0, "k": [ 200, 273, 0 ], "ix": 2, "l": 2 }, "a": { "a": 0, "k": [ 50.5, 36, 0 ], "ix": 1, "l": 2 }, "s": { "a": 0, "k": [ 100, 100, 100 ], "ix": 6, "l": 2 } }, "ao": 0, "ip": 0, "op": 48, "st": 0, "bm": 0 }, { "ddd": 0, "ind": 2, "ty": 2, "nm": "LOCK.png", "cl": "png", "refId": "image_1", "sr": 1, "ks": { "o": { "a": 0, "k": 100, "ix": 11 }, "r": { "a": 0, "k": 0, "ix": 10 }, "p": { "a": 0, "k": [ 200, 200, 0 ], "ix": 2, "l": 2 }, "a": { "a": 0, "k": [ 128, 195, 0 ], "ix": 1, "l": 2 }, "s": { "a": 0, "k": [ 100, 100, 100 ], "ix": 6, "l": 2 } }, "ao": 0, "ip": 0, "op": 48, "st": 0, "bm": 0 } ], "markers": [] }</issue_description>
Comments on the Issue (you are @copilot in this section)
@mattleibow Seems to also have issues with base64 images in the lottie files: https://lottiefiles.com/36895-healthy-or-junk-food @mattleibow Real issue tracked here: https://github.com/mono/SkiaSharp/issues/2173 @mattleibow Anyone have an example file like this? I am working on fixing this and I want to add some tests. @mattleibow This PR will get us to the happy place: https://github.com/mono/SkiaSharp/pull/2630 @mattleibow Now that we have https://github.com//pull/2...💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.