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

fix: improve perf of <Resize> by creating the canvas in a side effect #6874

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

stipsan
Copy link
Member

@stipsan stipsan commented Jun 7, 2024

Description

Currently everytime <Resize> renders it calls resize() before handing it off to the children prop callback.
Even though the init of canvas is memoized with useState, and it's only added to the dom once, in a useEffect, what happens during resize voids any benefits from these optimizations:

  • it sets canvas.height and canvas.width, causing layout trashing and repaint.
  • it calls ctx.drawImage(image, ...), which is expensive especially on larger images.

This PR changes it to run it in a single layout effect (the reasoning for why a layout effect is used instead of a regular effect is explained in code comments).
The effect only runs if the image, or maxHeight and maxWidth changes. Ensuring that these potentially expensive operations are only run when necessary. With the added bonus that React Compiler can now optimize the <Resize /> component

What to review

Check the perf, should be great, and otherwise behave like before.

Testing

Existing tests should cover potential regressions.

Notes for release

Images in the Hotspot & Crop tool now render faster, and uses far less memory.

Copy link

vercel bot commented Jun 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2024 3:08pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2024 3:08pm
test-compiled-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2024 3:08pm
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2024 3:08pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 13, 2024 3:08pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Jun 13, 2024 3:08pm

Copy link
Contributor

github-actions bot commented Jun 7, 2024

No changes to documentation

Copy link
Contributor

github-actions bot commented Jun 7, 2024

Component Testing Report Updated Jun 13, 2024 3:12 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 36s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 26s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 31s 11 7 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 14s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 8s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 52s 1 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 35s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 13s 20 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 3s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 7s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 20s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 15s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 29s 12 0 0

@stipsan stipsan force-pushed the refactor-image-tool-resize branch from 0510d1b to 75d5769 Compare June 10, 2024 09:21
Base automatically changed from refactor-use-async to next June 10, 2024 15:58
@stipsan stipsan marked this pull request as ready for review June 11, 2024 07:05
@stipsan stipsan requested a review from a team as a code owner June 11, 2024 07:05
@stipsan stipsan requested review from rexxars and removed request for a team June 11, 2024 07:05
@stipsan stipsan enabled auto-merge June 11, 2024 07:06
@stipsan stipsan force-pushed the refactor-image-tool-resize branch from 75d5769 to ae8e06b Compare June 11, 2024 08:03
@stipsan stipsan force-pushed the refactor-image-tool-resize branch from ae8e06b to 464b451 Compare June 12, 2024 01:43
@stipsan stipsan force-pushed the refactor-image-tool-resize branch from 464b451 to b4eb70d Compare June 12, 2024 18:02
Copy link
Contributor

@juice49 juice49 left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you!

@stipsan stipsan added this pull request to the merge queue Jun 14, 2024
Merged via the queue into next with commit 5d8031b Jun 14, 2024
41 of 43 checks passed
@stipsan stipsan deleted the refactor-image-tool-resize branch June 14, 2024 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants