Skip to content

Conversation

@ryan-di
Copy link
Member

@ryan-di ryan-di commented Aug 8, 2024

Previously, when an element is created, it's added to the scene and updated with informMutation, leading to an increase/change in scene version. This means that StaticCanvas, which gets re-rendered as the scene version updates, re-renders on every update happening to the new element. Given a large enough scene, we would see significant lags. And for elements like free draws, this kind of lag is perceived poorly.

This PR improves the drawing of new elements by

  • while a new element is still added to the elements array / scene, updating it does not trigger scene version change (so static canvas is not re-rendered);
  • and so to have the new element that's being created drawn, a canvas dedicated to the drawing of a new element is added

Implementation wise, building on the previous draggingElement -> newElement refactor, we have

  • newElement points to a new (non-selection) element that's being created
  • editingElement gets renamed to editingTextElement and is only used for a text that's being created/edited
  • when the creation of a new element is over (or when necessary), we reset states so that newElement is null and trigger scene updates so that the newly created element is rendered on the static canvas instead

Right now, this new element drawing improvement is only in the single player mode. Whether we should bring this feature over to collaboration mode remains to be seen.

@vercel
Copy link

vercel bot commented Aug 8, 2024

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

Name Status Preview Updated (UTC)
excalidraw ✅ Ready (Inspect) Visit Preview Aug 23, 2024 5:23pm
excalidraw-package-example ✅ Ready (Inspect) Visit Preview Aug 23, 2024 5:23pm
excalidraw-package-example-with-nextjs ✅ Ready (Inspect) Visit Preview Aug 23, 2024 5:23pm
1 Skipped Deployment
Name Status Preview Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Aug 23, 2024 5:23pm

@github-actions
Copy link

github-actions bot commented Aug 8, 2024

Coverage Report

Status Category Percentage Covered / Total
🔴 Lines 66.8% (🎯 70%) 58441 / 87477
🔴 Statements 66.8% (🎯 70%) 58441 / 87477
🟢 Functions 68.43% (🎯 68%) 1739 / 2541
🟢 Branches 80.97% (🎯 70%) 7304 / 9020
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
excalidraw-app/collab/Portal.tsx 53.51% 60.86% 54.54% 53.51% 43-46, 49-53, 56, 63-73, 78-80, 90-100, 110-117, 124-129, 134-138, 147-148, 185-199, 202-223, 226-247, 250-253
packages/excalidraw/appState.ts 98.26% 90% 71.42% 98.26% 18, 266-267, 274-275
packages/excalidraw/types.ts 100% 100% 100% 100%
packages/excalidraw/actions/actionFinalize.tsx 86.87% 90% 66.66% 86.87% 47, 63, 66-67, 112-121, 154-159, 211-219
packages/excalidraw/actions/actionHistory.tsx 97.72% 75% 100% 97.72% 45-47
packages/excalidraw/actions/actionProperties.tsx 78.14% 81.03% 87.69% 78.14% 197-199, 239-240, 274, 423-425, 441-450, 687, 773-799, 811-838, 864-865, 867-868, 880, 896-898, 939-962, 990, 1004, 1006-1010, 1016-1018, 1061-1066, 1068-1073, 1075-1135, 1164-1165, 1217, 1255-1256, 1305, 1330-1331, 1338, 1341, 1471-1503, 1555-1704, 1750-1751
packages/excalidraw/components/Actions.tsx 95.03% 93.42% 42.1% 95.03% 205-209, 282-283, 305-308, 314-317, 474-482
packages/excalidraw/components/App.tsx 71% 77.32% 70.98% 71% 483-484, 594-603, 702-703, 721-722, 743-803, 806-812, 825-828, 831-907, 910-929, 932-937, 945-955, 957-958, 963-964, 968-970, 984, 991-1253, 1313-1314, 1346-1348, 1355-1400, 1426, 1436, 1443-1446, 1455-1459, 1489-1490, 1568-1578, 1583-1598, 1602-1649, 1745-1750, 1779-1784, 1787-1817, 1825-1850, 1863-1954, 1957-1965, 1968-2027, 2030-2071, 2074-2079, 2116-2117, 2140-2141, 2172-2173, 2177-2178, 2198-2206, 2211-2224, 2230-2231, 2235, 2240-2248, 2250-2258, 2270, 2312-2313, 2335-2336, 2343, 2416-2418, 2421-2426, 2431-2432, 2472-2480, 2485-2494, 2532-2533, 2620-2621, 2625, 2628-2629, 2637-2640, 2649-2662, 2668-2671, 2674, 2676-2677, 2684-2685, 2691-2692, 2695-2696, 2704-2705, 2708-2709, 2720-2728, 2733-2734, 2785-2786, 2800-2806, 2812-2820, 2824-2832, 2836-2837, 2840-2873, 2876-2888, 2899-2900, 2911-2912, 2929-2933, 2937-2940, 2947-2949, 2967-2974, 2977, 2979-2984, 2988-2990, 3036-3037, 3044-3046, 3048-3071, 3097, 3103, 3159-3160, 3177-3179, 3201-3202, 3208-3211, 3217-3298, 3352, 3356, 3391-3392, 3451, 3461-3479, 3482-3488, 3491-3492, 3498-3514, 3619-3620, 3643-3657, 3662-3681, 3733-3734, 3814-3819, 3850, 3976-3977, 3979-3986, 4020-4024, 4026-4027, 4029-4032, 4057-4058, 4076-4078, 4080-4082, 4084, 4165-4168, 4190-4198, 4202-4204, 4214-4215, 4217-4237, 4244-4267, 4270-4276, 4293-4296, 4302-4305, 4315-4322, 4417-4421, 4425, 4430-4431, 4436-4440, 4473-4474, 4477-4478, 4490-4494, 4499-4500, 4506-4516, 4521-4548, 4553-4564, 4661-4662, 4746, 4771, 4797-4799, 4866-4867, 4885-4886, 5061-5062, 5065-5066, 5074-5075, 5125-5129, 5183-5190, 5196-5262, 5306, 5363, 5390, 5397-5398, 5411-5414, 5445, 5497-5500, 5503-5509, 5511-5518, 5535-5544, 5547-5548, 5678-5679, 5682, 5684-5689, 5695-5697, 5699, 5708, 5730-5735, 5737-5740, 5744-5745, 5755-5855, 5859-5860, 5875-5876, 5912-5918, 5945-5946, 5977, 5979-6006, 6013-6014, 6036-6037, 6056, 6058-6101, 6106-6107, 6109-6110, 6126-6127, 6133-6134, 6138-6141, 6144-6145, 6159-6185, 6193-6194, 6198-6201, 6203-6207, 6225-6229, 6276-6281, 6283-6285, 6301, 6303-6316, 6341-6344, 6388, 6405-6406, 6408-6431, 6446-6447, 6561-6589, 6695-6696, 6716-6717, 6811, 6817-6818, 6841-6860, 6875, 7023-7061, 7065-7115, 7130, 7139, 7173-7179, 7205-7207, 7373-7376, 7398-7428, 7441, 7443-7451, 7465, 7467-7475, 7482-7485, 7493-7498, 7525-7526, 7531-7533, 7536-7537, 7562-7563, 7594-7595, 7678-7679, 7726-7739, 7816-7819, 7845-7846, 7902, 7925-7931, 7942-7961, 7989-7995, 8027, 8069, 8075, 8090-8097, 8100-8107, 8157, 8235-8236, 8257-8259, 8262, 8276-8300, 8406-8419, 8442-8466, 8475-8514, 8527-8528, 8537-8544, 8562-8569, 8623-8649, 8651-8652, 8726-8727, 8734, 8736-8772, 8801, 8860, 8894-8896, 8921-8926, 8928-8929, 8934-8936, 8939-8959, 8973-8974, 8980-8989, 8994, 8998-9002, 9011-9015, 9018-9023, 9027-9034, 9064, 9066-9070, 9072-9082, 9098-9100, 9111-9119, 9123-9167, 9170-9241, 9249, 9267-9274, 9276-9292, 9307-9329, 9348-9350, 9396-9397, 9513-9517, 9519-9535, 9537-9541, 9553-9554, 9564-9580, 9599-9618, 9620-9621, 9648-9649, 9653-9654, 9664, 9666-9669, 9671-9672, 9712, 9753-9754, 9764, 9806, 9824-9832, 9853-9854, 9886-9889, 9982-9989, 10015-10016, 10059-10113, 10161-10162, 10171-10174, 10179-10180, 10201-10203, 10205-10209, 10247
packages/excalidraw/components/HintViewer.tsx 93.78% 92% 100% 93.78% 38-39, 59-60, 63-64, 105-106, 138-140
packages/excalidraw/components/canvases/InteractiveCanvas.tsx 88.13% 100% 100% 88.13% 86-113
packages/excalidraw/components/canvases/NewElementCanvas.tsx 96.42% 66.66% 100% 96.42% 25-26
packages/excalidraw/data/reconcile.ts 98.3% 71.42% 100% 98.3% 50-51
packages/excalidraw/element/dragElements.ts 77.28% 68.57% 100% 77.28% 40, 42-43, 50-51, 66-71, 199-223, 229-233, 238-258
packages/excalidraw/element/newElement.ts 92.17% 92.23% 85% 92.17% 143-148, 151-158, 178-192, 210, 358-359, 468, 474-475, 490-507, 544-545
packages/excalidraw/element/showSelectedShapeActions.ts 100% 100% 100% 100%
packages/excalidraw/renderer/interactiveScene.ts 87.17% 84.25% 85.71% 87.17% 85-86, 90-91, 137-138, 151-169, 253-264, 279-304, 378, 449-451, 472-473, 517-524, 576-587, 627-628, 679-680, 750-752, 807, 820-829, 847, 970-993
packages/excalidraw/renderer/renderNewElementScene.ts 89.39% 50% 66.66% 89.39% 44-45, 51-52, 61-63
packages/excalidraw/scene/Renderer.ts 98.8% 94.44% 100% 98.8% 85-86
packages/excalidraw/scene/selection.ts 87.04% 96.42% 88.88% 87.04% 62-71, 92-93, 102-121
packages/excalidraw/scene/types.ts 0% 0% 0% 0% 1-148
Generated in workflow #3138

@dwelle
Copy link
Member

dwelle commented Aug 13, 2024

@ryan-di Haven't had time to go through the code yet, but in collab it seems to break down (just being in the collab, no one else needs to be doing anything), where it's drawing the new element on both canvases or something.

Otherwise, when it works, it's great! Looking forward to shipping this! 🔥

@dwelle
Copy link
Member

dwelle commented Aug 23, 2024

In order to remove as much of the sceneNonce hacks as possible in anticipation of splitting it into sceneNonce & renderNonce so we have more control over what/when should rerender, I've remove cases where we skipped scene update / mutation on pointerdown/pointerup (8f95587 (#8340)) and in those cases it's not really necessary to optimize static scene updates, especially since on pointerdown we rendered anyway due to how memoizer for getRenderableElements works

Further, I've actually removed all the updateSceneNonce from Scene methods because it then turned out pretty much unused as all the other cases are using mutateElement() instead.

Copy link
Member

@dwelle dwelle left a comment

Choose a reason for hiding this comment

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

We're good to go! 🚀

@dwelle dwelle merged commit 5e1ff7c into master Aug 23, 2024
@dwelle dwelle deleted the ryan-di/new-element-perf branch August 23, 2024 18:27
aqandrew added a commit to aqandrew/excalidraw that referenced this pull request Sep 20, 2024
clarencechaan pushed a commit to clarencechaan/excalidraw that referenced this pull request Oct 3, 2024
Co-authored-by: dwelle <5153846+dwelle@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants