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

perf(frontend): MkImgWithBlurhashでblurhash描画に使うcanvasは再利用するようにする #10966

Merged
merged 6 commits into from
Jul 2, 2023

Conversation

tamaina
Copy link
Contributor

@tamaina tamaina commented Jun 6, 2023

Fix #10960

What

  • MkImgWithBlurhashでは、blurhash描画に使うcanvasは再利用するようにする
  • MkAvatarのimgをMkImgWithBlurhashに戻す。平均色のみ描画するようにした

Why

  • warningを消す
  • パフォーマンス向上?

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added the packages/frontend Client side specific issue/PR label Jun 6, 2023
@github-actions github-actions bot requested a review from syuilo June 6, 2023 06:01
@tamaina tamaina changed the title MkImgWithBlurhashで、blurhash描画に使うcanvasは再利用するようにする perf(frontend): MkImgWithBlurhashでblurhash描画に使うcanvasは再利用するようにする Jun 6, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #10966 (0ea5cfa) into develop (a1327fa) will increase coverage by 0.00%.
The diff coverage is 51.28%.

@@             Coverage Diff             @@
##           develop   #10966      +/-   ##
===========================================
  Coverage    77.34%   77.35%              
===========================================
  Files          739      907     +168     
  Lines        70536    91418   +20882     
  Branches      7103     7539     +436     
===========================================
+ Hits         54556    70716   +16160     
- Misses       15980    20702    +4722     
Impacted Files Coverage Δ
...ages/frontend/src/components/MkImgWithBlurhash.vue 66.40% <43.75%> (ø)
packages/frontend/src/workers/draw-blurhash.ts 64.70% <80.00%> (ø)
...ckages/frontend/src/components/global/MkAvatar.vue 97.82% <100.00%> (ø)

... and 165 files with indirect coverage changes

@acid-chicken
Copy link
Member

p1.a9z.dev 眺めてる限りではいい感じに動いてそう

Comment on lines +29 to +30
canvas.width = 64;
canvas.height = 64;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そういえばこれもっと大きくても良いのでは

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そう?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

サイズが大きくても buraha の速度はほぼ変わらない

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

保持しておくcanvasのサイズが大きいとメモリを食いまくるような気がする

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(表示に使うHTMLCanvasElementの話

@tamaina tamaina merged commit 734c41a into misskey-dev:develop Jul 2, 2023
slofp pushed a commit to Secineralyr/misskey.dream that referenced this pull request Jul 21, 2023
…skey-dev#10966)

* blurhashを描画するためのcanvasは再利用する

* Revert "perf(frontend): WebGL contextの数を減らす"

This reverts commit aeb8955.

* MkAvatarは平均色だけにする

* clean up

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v13.13.0において Too many active WebGL contexts.のWarningが多数表示される。
2 participants