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

Measure loop while scrolling in a scaled editor #1341

Closed
viorgu opened this issue Feb 21, 2024 · 6 comments
Closed

Measure loop while scrolling in a scaled editor #1341

viorgu opened this issue Feb 21, 2024 · 6 comments

Comments

@viorgu
Copy link

viorgu commented Feb 21, 2024

Describe the issue

The editor seems to get stuck in measure loops when scrolling while scaled and the loop is forcefully stopped with the "Measure loop restarted more than 5 times" check.

The issue can be easily reproduced adding a transform: scale(0.5); to the parent element of the editor on a demo page like: https://codemirror.net/examples/million/ and observing the console while scrolling fast or holding the Page Up/Down keys.
cm-measure-loop

From what I can tell the issue is that getBoundingClientRect() returns slightly different values while scrolling (probably because of rounding errors in BlockGapWidget height) and the scale computed in https://github.com/codemirror/view/blob/4e355eab50de94ab315ed293729f5365841fe3c8/src/viewstate.ts#L258 is also different so measuring continues at https://github.com/codemirror/view/blob/4e355eab50de94ab315ed293729f5365841fe3c8/src/editorview.ts#L427

Rounding the computed scale to 2 decimal places seems to prevent issue and should also remove the need for the other range checks in getScale.

An alternative fix would be to round the gap height at https://github.com/codemirror/view/blob/4e355eab50de94ab315ed293729f5365841fe3c8/src/docview.ts#L488 to an integer.

Unfortunately I'm not familiar enough with the project to know if the loss in precision from these changes would cause other issues or if there's a more appropriate fix.

I can open a PR with any of these options if it helps.

Browser and platform

MacOS & Windows / Firefox & Chrome

Reproduction link

No response

@marijnh
Copy link
Member

marijnh commented Mar 8, 2024

Indeed, that shouldn't be directly comparing floats like that. Attached patch adds a margin of .005 in which it doesn't treat the value as changed.

@viorgu
Copy link
Author

viorgu commented Mar 8, 2024

Would it make sense to check if the computed scale is 1 as well?
Otherwise it looks like there's a chance the scale might never reach 1 again if there's an animated reset.

@marijnh
Copy link
Member

marijnh commented Mar 8, 2024

Any scales close to 1 are already rounded to that in getScale, so I don't think this will be an issue.

@viorgu
Copy link
Author

viorgu commented Mar 8, 2024

I missed that, thanks!

@gquittet
Copy link

gquittet commented Apr 5, 2024

@marijnh The issue is still there when you use the scrollPastEnd plugin

marijnh added a commit to codemirror/view that referenced this issue Apr 7, 2024
FIX: Improve behavior of `scrollPastEnd` in a scaled editor.

Issue codemirror/dev#1341
@marijnh
Copy link
Member

marijnh commented Apr 7, 2024

@gquittet Can you provide a reproduction script for that? I couldn't get it to happen even with scrollPastEnd.

I did notice that extension didn't work right in a scaled editor, though. See attached patch.

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

No branches or pull requests

3 participants