Skip to content

Conversation

@tinyjin
Copy link
Member

@tinyjin tinyjin commented Sep 29, 2025

Replace existing base64 encoding pipeline with ThorVG's native asset resolver system.

This eliminates unnecessary encoding/decoding overhead and reduces memory usage when processing dotLottie image assets.

CleanShot 2025-10-03 at 15 17 57@2x

Asset Test Results


img1 img2 CleanShot 2025-10-03 at 14 40 03@2x

@tinyjin tinyjin requested a review from Copilot September 29, 2025 18:10
@tinyjin tinyjin self-assigned this Sep 29, 2025
@tinyjin tinyjin added the enhancement New feature or request label Sep 29, 2025
@changeset-bot
Copy link

changeset-bot bot commented Sep 29, 2025

⚠️ No Changeset found

Latest commit: 5dbf4df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements ThorVG's native asset resolver system to replace the existing base64 encoding pipeline for handling dotLottie image assets. This optimization eliminates unnecessary encoding/decoding overhead and reduces memory usage while maintaining the same functionality.

  • Replace base64 encoding/decoding with direct asset resolution through ThorVG's native API
  • Add asset resolver callback mechanism to handle image assets on-demand
  • Remove redundant base64 encoding implementation and related constants

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
dotlottie-rs/src/lottie_renderer/thorvg.rs Implements ThorVG asset resolver wrapper and integration with native API
dotlottie-rs/src/lottie_renderer/renderer.rs Adds asset resolver trait method to Animation interface
dotlottie-rs/src/lottie_renderer/mod.rs Defines AssetResolverFn type and adds asset resolver support to LottieRenderer trait
dotlottie-rs/src/fms/mod.rs Replaces base64 encoding with direct asset resolution method
dotlottie-rs/src/dotlottie_player.rs Integrates asset resolver callback with DotLottieManager for runtime asset loading

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@tinyjin tinyjin force-pushed the jinny/thorvg-asset-resolver branch 2 times, most recently from 8e150d0 to b4b139c Compare September 29, 2025 19:11
@hermet hermet requested a review from theashraf September 30, 2025 14:26
@tinyjin tinyjin force-pushed the jinny/thorvg-asset-resolver branch from 7ab1309 to 9845284 Compare October 3, 2025 05:41
Replace existing base64 encoding pipeline with ThorVG's native asset resolver system.

This eliminates unnecessary encoding/decoding overhead and reduces memory usage when processing dotLottie image assets.
dotLottie just introduced Font Assets Support
@tinyjin tinyjin force-pushed the jinny/thorvg-asset-resolver branch from 9845284 to 6cd55c9 Compare October 3, 2025 05:57
Fixed a bug in pre.29 where the asset resolver callback was not set
properly when running in non-threaded mode. Applied a temporary
hotfix commit for stability.

This commit will be removed in pre.30, as the proper fix will be
included in that release.
@tinyjin tinyjin force-pushed the jinny/thorvg-asset-resolver branch from 2da3760 to 5dbf4df Compare October 3, 2025 06:43
@hermet hermet marked this pull request as ready for review October 3, 2025 07:11
@hermet
Copy link
Member

hermet commented Oct 3, 2025

This must be applied with ThorVG v1.0-pre30.

@tinyjin
Copy link
Member Author

tinyjin commented Oct 3, 2025

Fyi. @theashraf
While I prepare this patch, I had removed b64 encoding but just reverted it again, since dotLottie just introduced v2.0 Font Asset.

For now ThorVG doesn’t support Font Asset Resolver, this patch would cover the image asset resolution pipeline as this PR aims.

We’ll soon support Font with asset resolver and will separately make patch for the Font.

@tinyjin
Copy link
Member Author

tinyjin commented Oct 3, 2025

Some unit tests failed in CI/CD, but they pass consistently in my local environment.
The failed cases also appear unrelated to my changes, so I assume this is a temporary issue.
Please let me know if this patch actually broke the tests.

PR Local
CleanShot 2025-10-03 at 16 19 11@2x CleanShot 2025-10-03 at 16 19 30@2x

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants