Improve scroll synchronization between editor and preview panes#201
Open
ExactDoug wants to merge 14 commits intomb21:masterfrom
Open
Improve scroll synchronization between editor and preview panes#201ExactDoug wants to merge 14 commits intomb21:masterfrom
ExactDoug wants to merge 14 commits intomb21:masterfrom
Conversation
Document root causes of split pane scroll synchronization issues on Windows, including O(n) reverse lookup, 30ms throttle jitter with Windows timer resolution, and DPI rounding inconsistencies. Includes recommended fixes prioritized by impact.
- Replace O(n) linear search with O(log n) binary search for reverse scroll map lookups, fixing severe performance issues with large docs - Switch from 30ms fixed throttle to requestAnimationFrame for frame-coupled updates, eliminating Windows timer jitter - Normalize rounding to Math.round() everywhere for consistency, fixing cumulative DPI drift from mixed Math.ceil/round These changes address the primary root causes of scroll sync issues on Windows, particularly with DPI scaling and large documents.
Add entry for editorPos=0, previewPos=0 to reverseScrollMapEntries. The loop started at i=1, causing the binary search to fail for positions near the top of the document.
Instead of returning just the lower bound from binary search, interpolate between the two nearest entries to calculate a more accurate editor position. This should eliminate the ~1/2 page offset that occurred when positions fell between known entries.
Temporary instrumentation to diagnose scroll sync issues: - Log scroll map build stats (sizes, sample entries) - Log editor→preview sync (input/output positions) - Log preview→editor sync (input/output positions) - Log interpolation details (entries, factor, result) This commit should be reverted after debugging is complete.
- Add scroll lock mechanism to prevent feedback loops where scrolling
one pane triggers the other pane to scroll back
- Track scroll source ('editor' or 'preview') and ignore events from
the pane that didn't initiate the scroll
- Clear lock after 50ms to allow normal scrolling to resume
- Clamp editor scroll position to valid scrollMap range to prevent
undefined mappings at document end
Replace absolute pixel position mapping with scroll percentage approach: - Calculate scroll percentage as: scrollTop / scrollableRange - Apply same percentage to the other pane's scrollable range - Both panes now reach their bottoms at the same time This fixes the issue where preview would reach bottom while editor was only 1/3 through the document, caused by different content heights and viewport sizes between the panes.
Combines best of both approaches: 1. Uses scroll map for line-based correlation, so headings (H1/H2/H3) and other elements with varying heights align properly 2. Scales the scroll map values to fit actual scrollable ranges, ensuring both panes reach their bottoms at the same time The scroll map provides content-aware positioning (e.g., a heading in the editor maps to the same heading in the preview), while the scaling ensures the overall scroll ranges match up.
Fix scaling instability caused by scroll map being rebuilt with different values. Instead of using maxPreviewInMap (which changes each rebuild), use the actual content heights: - previewScrollTo = (scrollMapValue / previewContentHeight) * scrollableRange - editorScrollTo = (editorPosInMap / editorContentHeight) * scrollableRange This provides a stable basis for scaling that doesn't change when the scroll map is rebuilt during scrolling.
The previous change to use previewContentHeight caused worse sync because scrollMapValue can exceed previewContentHeight (they come from different sources - getBoundingClientRect vs scrollHeight). Revert to using the scroll map's own maximum value for consistent scaling, as the scroll map values are internally consistent even if they don't match the actual content height.
Clean up console.log statements used during development and testing. The scroll synchronization improvements are complete for this iteration. Key improvements in this branch: - O(log n) binary search with interpolation (vs O(n) linear) - requestAnimationFrame throttling (vs 30ms fixed interval) - Scroll feedback loop prevention - Hybrid range normalization for consistent scrolling
The Preview→Editor direction was incorrectly scaling editorPosInMap by (editorPosInMap / maxEditorInMap) * editorScrollableRange, but the Editor→Preview direction uses editorScrollTop directly as an index without scaling. This asymmetry caused ~5% drift (e.g., 1800 → 1715 instead of 1800). Now both directions are symmetric: - Editor→Preview: input direct, output scaled - Preview→Editor: input scaled, output direct Also re-added debug logging for testing.
The editor scrollable range (17,422px) exceeded the scroll map size (16,615 entries), causing the preview to stay stuck at the bottom when scrolling up from the end of the document. Now both directions scale between actual scroll ranges and scroll map coordinates: Editor→Preview: - Scale editorScrollTop to scroll map index - Scale scrollMapValue to preview range Preview→Editor: - Scale previewScrollY to scroll map coordinates - Scale editorPosInMap back to editor range This ensures the full scroll range is utilized in both directions with perfect round-trip accuracy.
Final cleanup after implementing symmetric scaling fix. All scroll sync improvements are complete for this iteration.
Owner
|
Thanks for the PR. Did you test it yourself as well? And don't you think we still need some kind of throttle? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR significantly improves the scroll synchronization between the markdown editor (left pane) and the rendered preview (right pane), addressing longstanding issues particularly noticeable on Windows.
Key Improvements
Technical Details
The scroll map correlates editor line positions to preview element positions. Previously:
Now both directions use symmetric transformations:
Performance Impact
Testing
Tested on Windows with:
Known Limitations
The scroll map is line-based and cannot perfectly account for varying content heights (H1/H2/H3 headers, tables, images render at different sizes in editor vs preview). This is a fundamental limitation that would require a different approach to fully resolve.
Test Plan