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

During inspector scrubber interactions (slider control, the scrubbable labels of number controls) switch the inspector store to "real time mode" #6009

Closed
balazsbajorics opened this issue Jun 20, 2024 · 2 comments · Fixed by #6063
Assignees
Labels
Inspector Affects the inspector, settings, commands

Comments

@balazsbajorics
Copy link
Contributor

Problem: when dragging a slider control in the component section, or scrubbing a number control using the scrubbing interaction by dragging the label of the number control, the updates are instant on the canvas, but weirdly lag behind on the inspector.

This is casued by the inspector being wrapped in a LowPriorityStore.

Fix: we used to have a fix for this, but I don't remember what it was exactly.
The dispatch function takes a (now unread) DispatchPriority prop, which could be set to 'inspector'
Or alternatively, an Atom could be flipped that controls the inspector priority urgency.

@maltenuhn maltenuhn added the Inspector Affects the inspector, settings, commands label Jun 20, 2024
@seanparsons seanparsons self-assigned this Jun 21, 2024
@seanparsons
Copy link
Contributor

I've tried a few approaches but tellingly when I put an update directly to the input element itself it still felt laggy.

Some other discoveries:

  • For more complicated projects the building of the execution scope ends up being quite costly for each change as the require function changes with each change to the project contents.
  • The mutation/resize observers in the DOM walker run when there's any slack which means they end up running when there's any slight pause in the scrubbing which could be exacerbating any perceived lag.

@seanparsons seanparsons removed their assignment Jul 3, 2024
@balazsbajorics balazsbajorics self-assigned this Jul 10, 2024
@balazsbajorics
Copy link
Contributor Author

balazsbajorics commented Jul 10, 2024

okay so this is where I got so far:

  1. indeed horrible performance is making things bad, but
  2. weirdly React for some reason yields re-rendering of the slider control, even though in theory it doesn't use Deferred or any other low priority fiber setting, or anything like that. The slider component has a local state! which should update "real time" with the drag and the canvas, but for some reason the re-render is not queued. I need to look into more to understand why. I want to work a bit more on this because I feel like this reveals something about React I don't understand

balazsbajorics added a commit that referenced this issue Jul 11, 2024
**Problem:**
For projects with really bad performance, the slider control would oddly
lag behind the canvas and the number control.

![Before](https://github.com/concrete-utopia/utopia/assets/2226774/b2569e46-1712-4b86-b35a-f96a3fd54f18)
_Note how the slider control doesn't update while the number control and
the canvas do update_

**Diagnosis:**
The slider control uses a local state to store the drag's current value.
Probably because our dispatch internally uses a flushSync, this local
state update was treated with a lower priority in React, and because the
frame times are egregious, it never got enough idle time for a
requestIdleCallback.

**Fix:**

![After](https://github.com/concrete-utopia/utopia/assets/2226774/55bc2a27-b311-4d76-90bf-0caa0110ec25)
_note how the slider updates the same time as the number control and the
canvas_

By wrapping the local state update into a flushSync block, I managed to
force it to update with the same priority as the rest of the editor.
**Note:** The frame times are still egregious, but that's a different
problem.

Fixes #6009
liady pushed a commit that referenced this issue Dec 13, 2024
**Problem:**
For projects with really bad performance, the slider control would oddly
lag behind the canvas and the number control.

![Before](https://github.com/concrete-utopia/utopia/assets/2226774/b2569e46-1712-4b86-b35a-f96a3fd54f18)
_Note how the slider control doesn't update while the number control and
the canvas do update_

**Diagnosis:**
The slider control uses a local state to store the drag's current value.
Probably because our dispatch internally uses a flushSync, this local
state update was treated with a lower priority in React, and because the
frame times are egregious, it never got enough idle time for a
requestIdleCallback.

**Fix:**

![After](https://github.com/concrete-utopia/utopia/assets/2226774/55bc2a27-b311-4d76-90bf-0caa0110ec25)
_note how the slider updates the same time as the number control and the
canvas_

By wrapping the local state update into a flushSync block, I managed to
force it to update with the same priority as the rest of the editor.
**Note:** The frame times are still egregious, but that's a different
problem.

Fixes #6009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Inspector Affects the inspector, settings, commands
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants