Console: Prompt flickers on execute

[seeM]

Frame before execute:

Immediately after execute:

Following that:

[softwarenerd]

I am not seeing this. Here is a screen recording of your example slowed to 25%:

https://github.com/rstudio/positron/assets/853239/b75d93df-724d-4ac9-824b-03d732d2aa47

You can scrub through this.

[seeM]

I see it happening in your video, but only on the last try. You have to scrub reeeeeally slowly.

[seeM]

I played your video with the playback speed set to 25% (so 6.25% playback speed overall since yours is already slowed down) and recorded that section:

https://github.com/rstudio/positron/assets/559360/76876034-166e-4845-b34c-ad6f1fb7be70

[seeM]

I did a little bit of digging into this, and found two timing/ordering issues:

1. There may be a delay between when we clear the code editor widget (immediately after executing its value, source), and when we display the executed input in the history (when the execute_input is received and an activity item is created, source). These should render in the same frame.
2. We may disable the input prompt (on receiving a status: busy message, source) before we display the executed input in the history.

I don't have any good ideas on how to fix it though.

[softwarenerd]

Please see https://github.com/rstudio/positron/pull/916 for the fix for this.

[seeM]

Thank you Brian! It seems to be fixed after your PR, and your approach makes a lot more sense than my hack.

[seeM]

It seems that that are still single-frame flickers happening. It's really hard to see (although unfortunately my eyes are sensitive to this sort of thing) so I'll demonstrate using images below instead of a video.

My understanding from looking into this before is that the issue is around synchronizing rendering between our React components and VSCode's CodeEditorWidget. I found that the hack of wrapping our state updates in a setTimeout(..., 0) fixed the issue (diff) but hopefully there's a better solution!

Frame 1

As I press enter. This is correct.

Frame 2

Issue 1: the console input is cleared before the activity item is added

Frame 3

Issue 2: the prompt (>>>) shows even though we're waiting for output from the kernel

Frame 4

This is the correct and expected end state.

[petetronic]

Moving this to after the Alpha milestone

[seeM]

I discovered the problem: React and Monaco renders are not synchronized. So when we do codeEditorWidget.setValue('foo') and immediately set some React state, the code editor widget is rendered, but React often renders 1-2 frames later (possibly due to batching state updates?).

One way to synchronize them is to pull methods that cause Monaco renders into useEffects. For example, instead of directly calling:

codeEditorWidget.setValue('foo')

We would track the editor widget's value as state, configure an effect that renders the widget when the state changes, and then update the state when needed:

// Track the editor widget's value as state.
const [codeEditorWidgetValue, setCodeEditorWidgetValue] = useState<string>();

// Setup an effect that actually renders the editor widget with the new value when the state is changed.
useEffect(() => {
    if (codeEditorWidgetRef.current && codeEditorWidgetValue !== undefined) {
        codeEditorWidgetRef.current.setValue(codeEditorWidgetValue);
    }
}, [codeEditorWidgetRef, codeEditorWidgetValue]);

// ... Then set the state when needed.
setCodeEditorWidgetValue('foo');

The result is that the editor widget render will happen immediately after React renders. In my testing, this fixes the render order 100% of the time – no flickers.

One issue with this approach is that if we move one editor widget method to a state and effect, we'll need to do the same for any other methods called at the same time, otherwise we end up in the unsynchronized situation again.

The issue may be a lot simpler to address if we wrap the widget in a React component that takes things like the value as props (basically what react-monaco-editor does), but I'm not sure what other unforeseen issues that may open up.

Alternatively, it would still be possible to keep most of the current structure, and use the pattern from above where needed. But devs would need to be careful not to set editor widget React state and call an editor widget render method at the same time.

[testlabauto]

Verified Fixed

Positron Version(s) : 2024.07.0-97
OS Version          : OSX

Test scenario(s)

I tried many times and was never able to observe the flicker.

Link(s) to TestRail test cases run or created: N/A