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

Add Kotlin encoder #55

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

hearsilent
Copy link

@hearsilent hearsilent commented Apr 27, 2020

#41

@hearsilent
Copy link
Author

@hangduykhiem Please review this 🙏

@hangduykhiem
Copy link
Collaborator

Sorry this got lost in my inbox. Will review ASAP.

@connyduck
Copy link
Contributor

I tried this with the Mona Lisa and the result is not as expected 🤔

@hearsilent
Copy link
Author

@connyduck In demo app the ratio is 20:12 (BlurHashDecoder.decode(etInput.text.toString(), 20, 12))
You should change this to your image ratio.

@connyduck
Copy link
Contributor

changing the ratio only stretches the gradient

@hangduykhiem
Copy link
Collaborator

hangduykhiem commented Jun 21, 2020

Yup, I tried with both config and the result is similar to Connyduck's finding. For comparison, this is the result of web encoder

image

And the string is LHC6SkxEElE4_JWCoyWCTIbHxsbH.

Edit: Here's one more comparison so that we can figure out where the bug is. The first one on blurha.sh decoded to LGF5]+Yk^6#M@-5c,1J5@[or[Q6, while your solution returns UEHV6nfQ2KfQg9fQfQfQkVfQaefQfQfQjtjt

@hangduykhiem
Copy link
Collaborator

hangduykhiem commented Jun 21, 2020

Actually now that I think of it, @DagAgren's vision is that encode and decode are separate files that can be copy directly to the user directory.

BlurHashDecode.swift and BlurHashEncode.swift contain a decoder and encoder for BlurHash to and from UIImage. Both files are completeiy standalone, and can simply be copied into your project directly.

Can you make it so that each of them are self contained? Else I think we might need to move the whole thing to bintray or somewhere else to make it easier for others.

@hangduykhiem
Copy link
Collaborator

hangduykhiem commented Jun 21, 2020

While the decoder itself is pretty much self tested @ignaciotcrespo introduced some nice test for decoder in #66 as well, I think the encoder needs some simple unit test for better verification of result.

I'm thinking of something like comparing the output of the first image with LEHV6nWB2yk8pyo0adR*.7kCMdnj and the mona lisa image with LHC6SkxEElE4_JWCoyWCTIbHxsbH. It should be easy to write and maintain, while catching the problem if we ever need to touch the encoder.

@hangduykhiem
Copy link
Collaborator

All being said, thank you for the PR, @hearsilent. I meant to do this a while ago, but... 😅

@hearsilent
Copy link
Author

@connyduck @hangduykhiem I've fixed encode with wrong result. It cause by encodeAC with wrong pow method.

@hearsilent
Copy link
Author

The result is like this:
image

This result is generated by BlurHashEncoder.encode(bitmap, 4, 3)

@moritzruth
Copy link

Can you please merge this? I would like to use it, but I'll use https://github.com/hsch/blurhash-java for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants