Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lottie support, but very raw state #134

Merged
merged 13 commits into from
Oct 12, 2020
Merged

Lottie support, but very raw state #134

merged 13 commits into from
Oct 12, 2020

Conversation

wh1t3lord
Copy link
Contributor

@wh1t3lord wh1t3lord commented Oct 12, 2020

Need to be fixed:

  1. Properly load file (idk how to fix that, because we operate json file), hardcoded file path for default loading.
  2. Create sample project. Temporary I changed loaddocument project and it loads lottie.rml file as example.
  3. Color interpretation.
  4. Check cmake file and properly connect library (hardcoded path it's not right solution)
  5. Fix updating texture without allocation every frame a new texture (just write information, but don't needed to allocate physically "new" texture if resize haven't done)
  6. Updating attributes, like 'src', 'width', 'height'.
  7. Control playing and handle playing stuff. Maybe it needs to be fixed by FPS's application or framerate of animation.

Done:

  1. Connected lottie as submodule
  2. Working lottie library
  3. Rendering
  4. Custom tag

I am done here, and can't support further (maybe I can add fixes for color interpretation sometime though).

@mikke89
Copy link
Owner

mikke89 commented Oct 12, 2020

Thanks for this! Will need some changes before we merge into master. But I can take over from here.

Some changes I intend to to before merging (in addition to your list):

  • Move the element out of the core library, add it as a plugin in the sample project.
  • Will not use submodules, rather let users handle dependencies themselves.

@mikke89 mikke89 changed the base branch from master to lottie October 12, 2020 15:06
@mikke89 mikke89 merged this pull request into mikke89:lottie Oct 12, 2020
@mikke89 mikke89 added the enhancement New feature or request label Oct 12, 2020
@rokups
Copy link
Contributor

rokups commented Oct 12, 2020

I see this PR tried to add a new element. Sure it is a good idea? wouldnt something like a custom decorator make more sense? or even: decorator: image(anim.lottie)?

@mikke89
Copy link
Owner

mikke89 commented Oct 12, 2020

There will be no changes to the core library, as I intend to move that element out and into the sample as a plugin, so it won't do anything unless you manually copy over the plugin. Does that ease your concern? :)

Otherwise, I'm willing to discuss everything before I merge it to master.

@mikke89
Copy link
Owner

mikke89 commented Oct 12, 2020

By the way, @DiamondHat, do you have a source and a license for the lottie animation you included?

@rokups
Copy link
Contributor

rokups commented Oct 12, 2020

I do not really have a preference where integration lives. Just in what form it manifests. But maybe there is no cause for concern. We have <img> and its fine.. I no longer know which would be better :)

@mikke89
Copy link
Owner

mikke89 commented Oct 12, 2020

I think it makes more sense as an element, rather than a decorator, especially since it has intrinsic size. Plus, I think this is the easiest way to update it regularly.

@wh1t3lord
Copy link
Contributor Author

@mikke89 I used json animations from rlottie repo, and guessed it's all by MIT license. I don't think they are used commercial animations to publish them by MIT license xD

But you can check this file https://github.com/Samsung/rlottie/blob/master/COPYING

@wh1t3lord wh1t3lord deleted the lottie branch October 13, 2020 08:02
mikke89 added a commit that referenced this pull request Oct 17, 2020
The lottie element has been refactored into a separate sample, injected as a plugin.

Co-authored-by: Michael R. P. Ragazzon <mikke89@users.noreply.github.com>
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.

3 participants