Skip to content

Fix softwrap scrolling issues#1981

Merged
zyedidia merged 2 commits intomicro-editor:masterfrom
dmaluka:softwrap-improvement
Apr 7, 2021
Merged

Fix softwrap scrolling issues#1981
zyedidia merged 2 commits intomicro-editor:masterfrom
dmaluka:softwrap-improvement

Conversation

@dmaluka
Copy link
Collaborator

@dmaluka dmaluka commented Jan 7, 2021

Softwrap implementation enhanced to fix various issues with scrolling, centering, relocating etc.

The main idea is simple: work not with simple line numbers but with (Line, Row) pairs, where Line is a line number in the buffer and Row is a visual line (a row) number within this line. The logic remains mostly the same, but simple arithmetic operations on line numbers are replaced with corresponding operations on (Line, Row) pairs.

Fixes #632, #1657

This is the first step in improving the soft wrapping functionality. I think the next steps should be:

Softwrap implementation enhanced to fix various issues with scrolling,
centering, relocating etc.

The main idea is simple: work not with simple line numbers but
with (Line, Row) pairs, where Line is a line number in the buffer
and Row is a visual line (a row) number within this line.
The logic remains mostly the same, but simple arithmetic operations
on line numbers are replaced with corresponding operations on
(Line, Row) pairs.

Fixes micro-editor#632, micro-editor#1657
v.StartLine = h.Buf.LinesNum() - v.Height
h.SetView(v)
}
v.StartLine = h.Scroll(h.SLocFromLoc(h.Buf.End()), -v.Height+1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The use of v.Height here (and in other functions in this file) is buggy, since the actual buffer height is v.Height-1 if statusline is on. This is a separate issue.

v := LogBufPane.GetView()
endY := buffer.LogBuf.End().Y

if endY > v.StartLine+v.Height {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This check is redundant since StartLine has been already properly updated by CursorEnd.

// You might think that this is obviously just v.StartLine + v.Height
// but if softwrap is enabled things get complicated since one buffer
// line can take up multiple lines in the view
func (w *BufWindow) Bottomline() int {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Bottomline removed because its use is buggy anyway: if the bottom line is wrapped, Bottomline doesn't tell which row is at the bottom.
Instead of w.Bottomline() use w.Scroll(v.StartLine, height-1).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BTW this Bottomline implementation is inconsistent: if we've scrolled past the last line, it returns different values with softwrap=off and softwrap=on (which results in different behavior).

ret = true
} else if cy >= b.LinesNum()-scrollmargin && cy >= height {
w.StartLine = b.LinesNum() - height
} else if c.GreaterThan(w.Scroll(bEnd, -scrollmargin)) && c.GreaterThan(w.Scroll(w.StartLine, height-1)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that I've changed cy >= height to essentially cy >= w.StartLine + height. This, together with the removal of Bottomline, fixes some bugs with scrolling past the last line.

@zyedidia
Copy link
Member

zyedidia commented Jan 8, 2021

Cool! This has been a sore point for a while, so it will be good to fix it. Does SLoc track visual x,y locations?

@dmaluka
Copy link
Collaborator Author

dmaluka commented Jan 8, 2021

Does SLoc track visual x,y locations?

Not quite yet. It tracks the vertical position only. The visual y can be easily obtained from a sloc (it equals w.Diff(w.StartLine, sloc)) but the visual x cannot.

However I've been already thinking about adding one more struct:

type VLoc struct {
	SLoc
	VisualX int
}

and functions VLocFromLoc (similar to SLocFromLoc) and LocFromVLoc. It should be easy to implement and it would allow any location in the buffer to be represented as a location in the linewrapped buffer. In particular, it would be useful for implementing moving cursor up and down within a wrapped line. Perhaps it would also allow to significantly simplify the code of LocFromVisual.

@ghost
Copy link

ghost commented Jan 8, 2021

In particular, it would be useful for implementing moving cursor up and down within a wrapped line.

This would be awesome!

@dmaluka
Copy link
Collaborator Author

dmaluka commented Mar 17, 2021

On my softwrap-improvement-dev branch I've implemented a bunch of further softwrap improvements on top of this PR, including cursor up/down movement within wrapped line, word wrapping, and fixes for issues #645 and #1979.

@zyedidia zyedidia merged commit c5798b5 into micro-editor:master Apr 7, 2021
@dmaluka
Copy link
Collaborator Author

dmaluka commented Apr 29, 2021

On my softwrap-improvement-dev branch I've implemented a bunch of further softwrap improvements on top of this PR, including cursor up/down movement within wrapped line, word wrapping, and fixes for issues #645 and #1979.

For reference, those changes are now in my PR #2076.

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.

View doesn't center on cursor/current line with softwrap

2 participants