Add telegram skin recolor

[yuraiz]

This PR implements Animation::from_data_tg function that takes FitzModifier (one of emoji skin tones) as last argument

[msrd0]

Hm ... I don't like this. It makes claims about the internal structure of the C binding's struct - which has an additional field in the samsung version for example.

[yuraiz]

We need to use this struct only when telegram's version of library is used, so I think it's normal

also rust bindings uses only first field of this struct

// That works on my pc
struct Lottie_Animation_S {
    std::unique_ptr<Animation> mAnimation;
    // other values
};

Ofc I can make fork on teleram's rlottie and update c bindings but I don't know what repo use for pull request

[msrd0]

also rust bindings uses only first field of this struct

Well, C++ is going to instantiate a struct with your definition and access and drop a struct with the rlottie libraries version. Given that the telegram version doesn't even do releases, and likely does not commit to keep that struct the same forever, this sounds like a bad idea.

Ofc I can make fork on teleram's rlottie and update c bindings but I don't know what repo use for pull request

It'd be best if you could get the folks at telegram to expose some C API in addition to what they currently have to change the skin tone. Not sure if that's something they're willing to do.

[yuraiz]

If I understand correctly TelegramSwift also uses struct with only unique_ptr
https://github.com/overtake/TelegramSwift/blob/master/submodules/RLottie_Xcode/Sources/RLottieBridge.mm#L17=L21
but it doesn't use c bindings

[yuraiz]

I suggest to use SkinTone enum like this, because not everyone knows about fitzpatrick skin type
Also I want to use custom enum instead of auto generated because it works better with code analysis and autocompletion

#[cfg(feature = "tg")]
pub enum SkinTone {
	None = 0,
	Light,
	MediumLight,
	Medium,
	MediumDark,
	Dark
}

#[cfg(feature = "tg")]
impl SkinTone {
	pub fn from_emoji_modifier(emoji_modifier: char) -> Self {
		match emoji_modifier {
			'\u{1F3FB}' => Self::Light,
			'\u{1F3FC}' => Self::MediumLight,
			'\u{1F3FD}' => Self::Medium,
			'\u{1F3FE}' => Self::MediumDark,
			'\u{1F3FF}' => Self::Dark,
			_ => Self::None
		}
	}
}

[msrd0]

Yes, that sounds like a good API design. Maybe don't have a _ => Self::None case though as that implies that literally any char has a meaning? I wouldn't expect for an input like ✌a for a to be an skin tone modifier.

Also, I added a CI test for the telegram version of rlottie, please update this PR.

Regarding the C struct, I'm still not sure if I'm happy with that ...

[yuraiz]

I wouldn't expect for an input like ✌a for a to be an skin tone modifier.

I want to put last char of my message text to get skin color

https://github.com/overtake/TelegramSwift/blob/13d62c1c03df7dea70bad0f395fb68115f5a0b43/Telegram-Mac/CoreExtension.swift#L426-L432

[yuraiz]

Maybe I can use Option<SkinTone> instead

[yuraiz]

Lol, I already have fitzpatrick_type in Animated emoji

[yuraiz]

We with @melix99 decided keeping a fork
so I'll extend c binidgs in that fork with requred methods later

https://github.com/melix99/rlottie

[yuraiz]

That works, but I haven't published my rlottie changes yet

[yuraiz]

I made fitzpatrick_type of type i32 'cause that type is used in tdlib-rs

[msrd0]

Sorry for the delay on this PR.

Is your intention to merge the changes on your fork into the telegram rlottie library? I really don't think it would be a good idea to have a 3rd version of the rlottie library around ...

I made fitzpatrick_type of type i32 'cause that type is used in tdlib-rs

Sorry but that doesn't make any sense. It is an enum in the rlottie library so why wouldn't we just use that?

[yuraiz]

Sorry but that doesn't make any sense. It is an enum in the rlottie library so why wouldn't we just use that?

https://docs.rs/tdlib/latest/tdlib/types/struct.AnimatedEmoji.html#structfield.fitzpatrick_type

It will be easier to pass that variable without any conversions

[msrd0]

Well, someone has to do the conversion, and I think it would make more sense for the rlottie binding to expose an API the way rlottie does it.

I think without any changes to the upstream rlottie library, it would make the most sense to, if some skin-recolor feature is enabled, use a small header and c++ file to expose the rlottie::FitzModifier enum and a method that allows passing it to rlottie::Animation::loadFromData.

[yuraiz]

Is your intention to merge the changes on your fork into the telegram rlottie library

TelegramMessenger/rlottie#4 haven't merged yet, also other telegram clients don't care about c bindings, telegram for macos also keeps its own fork

[msrd0]

TelegramMessenger/rlottie#4 haven't merged yet

Yes I know, I even use that PR in my CI (https://github.com/msrd0/install-rlottie-action/blob/v1/action.yml#L34).

It is very sad to see that no one seems to be maintaining rlottie in a way that people can contribute, but at least it's open source. To me, since I want this library to be as widely compatible as possible, it would make the most sense to:

• With default features, keep it the way it is. That makes it work with samsung and telegram versions, but does not allow skin recolouring.
• Add a feature as I said before, that keeps using the existing c api but adds just one more function to allow skin recolouring.

Also, docs.rs uses the 0.0.1 release from samsung to generate the documentation, since that's the only one available in the ubuntu packages.

[yuraiz]

With default features, keep it the way it is. That makes it work with samsung and telegram versions, but does not allow skin recolouring.

I've added telegrand feature, so any things dependent on melix99/rlottie will use that feature

[yuraiz]

I don't want to add a SkinTone enum because it allows to recolor only telegram emojis, and I don't sure that it will be used anywhere except telegrand

[msrd0]

I just searched through the rlottie code and it seems to be the case that there can only be those variants: https://github.com/TelegramMessenger/rlottie/blob/master/src/lottie/lottieparser.cpp#L643

Since the rlottie code kinda forces the enum, I think we should keep that in our binding.

[msrd0]

I'm going to close this PR as it hasn't received any updates within the last 3 years