-
Notifications
You must be signed in to change notification settings - Fork 92
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
feat(NcBlurHash): Add a blur hash component #6396
Conversation
/backport to next |
5de89ec
to
5bd6000
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it should be a part of this component, but when used, this element must have exactly the same aspect ratio as the original image. And when the image is loaded, it should be replaced with the image.
Having all that it seems to me, a developer still has to implement manually even more, than this component does.
Could you link places where you use this component? To see if we can reuse more.
return | ||
} | ||
|
||
const { height, width } = canvas.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken, 32x32 is enought for the blurhash. Then we can just use CSS for scaling it.
cc @Antreesy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I remember it, with different dimensions a blurred thumbnail was quite distorted and different from original image, than 32*32 canvas. At least that's what upstream lib is using in demo:
https://blurha.sh/
nextcloud/spreed@0fe0f01
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be a linear interpolation of a gradient, so scaling could lead to pixeled output if the image is rather big (nor sure how smart browsers scale images).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using the canvas size is the way to go, as we do not always render square images. As long as the original image ratio is preserved, then it is fine.
Not sure as this can also lead to overengineering. The user could have different use cases (maybe its not a placeholder but just the preview?), so what I could imagine: |
e1c4c5b
to
e9ff77f
Compare
@ShGKme I pushed a commit implementing that feature Bildschirmaufnahme_20250120_222940.webm |
For all that want to check the preview netlify: In the meantime you can just append |
return | ||
} | ||
|
||
const { height, width } = canvas.value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using the canvas size is the way to go, as we do not always render square images. As long as the original image ratio is preserved, then it is fine.
6aceb4b
to
4179bb6
Compare
@artonge but it is not the canvas or image, but blurhash render size |
Blurhash render size should keep the same ratio as the original image size, no? |
It is, because the blur hash is just (more or less) an encoded gradient. So I think the correct way to go is to render the hash on the canvas size. |
6d5bd54
to
a255c43
Compare
BTW Wolt (the author of that algorithm) also does it like this in their react implementation: |
a255c43
to
42a22dd
Compare
This allows to include image previews / placeholders in apps. Currently something similar is used for the files and for the photos app. So this can be used to de-duplicate code. Co-authored-by: Grigorii K. Shartsev <[email protected]> Co-authored-by: Ferdinand Thiessen <[email protected]> Signed-off-by: Ferdinand Thiessen <[email protected]>
42a22dd
to
0ee2517
Compare
The backport to # Switch to the target branch and update it
git checkout next
git pull origin next
# Create the new backport branch
git checkout -b backport/6396/next
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 0ee2517b
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/6396/next Error: No changes found in backport branch Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
/backport to next |
The backport to # Switch to the target branch and update it
git checkout next
git pull origin next
# Create the new backport branch
git checkout -b backport/6396/next
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 0ee2517b
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/6396/next Error: No changes found in backport branch Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
import PQueue from 'p-queue' | ||
|
||
const queue = new PQueue({ concurrency: 5 }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? 🤔
☑️ Resolves
This allows to include image previews / placeholders in apps.
Currently something similar is used for the files and for the photos app. So this can be used to de-duplicate code.
🖼️ Screenshots
🏁 Checklist
next
requested with a Vue 3 upgrade