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.

Sign up for GitHub

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

Jump to bottom

enhance(client): Compress non-animated PNG files #9334

Merged
merged 9 commits into from
Dec 18, 2022
Merged

enhance(client): Compress non-animated PNG files #9334

merged 9 commits into from
Dec 18, 2022

Conversation

saschanaz
Copy link
Member

Fixes #9277

What

  1. Adds a dependency is-file-animated that parses image header from a Blob to detect whether it's an animated image.
  2. Splits compression configuration part as compress-config.ts from packages/client/src/scripts/upload.ts
  3. Adds a client unit test for compress-config.ts, with additional setup to run Jest for client

Why

  1. To prevent losing animation by falsely compressing APNG to JPEG.
  2. To write a unit test without importing everything (that depends on browser features or requires Jest transformer, e.g. Vue files)
  3. To make sure nothing breaks by my bad code. I had to setup Jest myself as I couldn't find existing tests for client.

Additional info (optional)

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Dec 17, 2022
@codecov
Copy link

codecov bot commented Dec 17, 2022 •
edited
Loading

Codecov Report

Merging #9334 (244956f) into develop (9c5dfd2) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #9334      +/-   ##
===========================================
- Coverage    22.48%   22.48%   -0.01%     
===========================================
  Files          700      700              
  Lines        65486    65489       +3     
  Branches      2120     2120              
===========================================
  Hits         14723    14723              
- Misses       50763    50766       +3
Impacted Files Coverage Δ
...ackages/backend/src/server/api/ApiServerService.ts 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

jest周りは分離してPRを出してください

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

本音としてはwasm-vipsでバックエンドと同じ結果を得たいけど、SafariがSIMDに対応してないのでダメということらしい

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022 •
edited
Loading

PNG to PNG、やっぱりサイズが大きくなってしまわないかしら

(縮小の時だけ処理するとかがいいのかも

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

しかしpngをjpegに圧縮するのは許し難い

@saschanaz
Copy link
Member Author

PNG to PNG、やっぱりサイズが大きくなってしまわないかしら

(縮小の時だけ処理するとかがいいのかも

なんかややこしくなりそうですが、一斉quality 95 WebPにしたらどうですか

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

webp

  • Mastodon v3が対応してない
  • Safariはqualityを無視する

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

送信サイズの削減だけをねらうならいっそのことwasm-avif でavif にしちゃった方が良さそう

@saschanaz
Copy link
Member Author

圧縮の後のサイズチェックを追加してみました
こうしたらなぜかもっと大きくなったJPGにも対応できます

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

圧縮の後のサイズチェックを追加してみました

こうしたらなぜかもっと大きくなったJPGにも対応できます

良さそう

@tamaina tamaina changed the title Compress non-animated PNG files enhance(client): Compress non-animated PNG files Dec 17, 2022
@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

is-file-animatedってavif対応しないのかしら？

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

/deploy sha=d1fce3664cc9cb9396643463ec3651d804bcee4e

@github-actions
Copy link
Contributor

Your preview environment pr-9334-syuilo has been deployed.

Preview environment endpoint is available here

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

圧縮前か圧縮後のどちらを使ったかをconsole.logで知りたい (if (DEV)で)(風呂に入るけど)

@saschanaz
Copy link
Member Author

もっと情報をロギングするようにしました。JPGは圧縮したら悪くなる場合が多いことが判明。 PNGは意外と悪くなかったです

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

/deploy sha=163b5e4

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

ブラウザによって圧縮率が異なるかもしれない

@saschanaz
Copy link
Member Author

ふむ。PNGもJPGもChromeでもうちょっと小さくなることがわかりました

どっちも圧縮したら悪くなる場合が少なくないのでこれは役に立つかも

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

…ところで: 今のままだとNODE_ENVが強制的にproductionにされてしまう

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

vite build --mode developmentしなければならないんだけど、Windows環境のことを考えるとどうにもならんような気がする

@saschanaz
Copy link
Member Author

saschanaz commented Dec 17, 2022 •
edited
Loading

vite build --mode developmentしなければならないんだけど、Windows環境のことを考えるとどうにもならんような気がする

Windowsで yarn dev したらちゃんとdev modeで動きます

（でもなんかbackendビルド終わってないのにbootしようとします）

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022 •
edited
Loading

Windowsで yarn dev したらちゃんとdev modeで動きます

yarn devはvite build --watch --mode developmentだから…まあローカルはこれでいいのか

(okteto環境でもdevelopmentで動いて欲しいという話)

（でもなんかbackendビルド終わってないのにbootしようとします）

通常運行

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

yarn devのvite build --watchは妥協案で、やっぱりHMRで動かしたいと思っている

@tamaina tamaina requested a review from syuilo December 17, 2022 14:46
@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

一応しゅいろにレビューリクエストするか

@tamaina
Copy link
Member

tamaina commented Dec 17, 2022

p1.a9z.devでテスト中

@syuilo syuilo merged commit a47d172 into misskey-dev:develop Dec 18, 2022
@syuilo
Copy link
Member

syuilo commented Dec 18, 2022

👍

@saschanaz saschanaz deleted the is-animated branch December 18, 2022 07:32
This was referenced Dec 19, 2022
Open
Closed
yu256 pushed a commit to yu256/akatsukey that referenced this pull request Mar 5, 2023
@saschanaz @tamaina @yu256
enhance(client): Compress non-animated PNG files (misskey-dev#9334)
db77abd 
* style: fix TS lint errors about `ev.target`

* enhance: compress non-animated PNG

* PNG to PNG?

* defer jest things (add it later)

* Delete jest.config.cjs

* check the compressed file size

* log compression stats

* use ??

* handle if ($i == null)

Co-authored-by: tamaina <tamaina@hotmail.co.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tamaina tamaina tamaina approved these changes

@syuilo syuilo Awaiting requested review from syuilo

Assignees
No one assigned
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a way to compress PNG images when uploading
3 participants
@saschanaz @tamaina @syuilo