WIP: rough cut of ScrollPanel --- LANTERNA-491

[ginkoblongata]

Creating this PR early before completion in case there is any comments which can gather.

[ginkoblongata]

testing with this:

$ mvn clean install ; java -cp "target/lanterna-3.1.0-SNAPSHOT-tests.jar:target/lanterna-3.1.0-SNAPSHOT.jar" -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 com.googlecode.lanterna.issue.Issue490 --mouse-move

[ginkoblongata]

I rebased this branch onto release/3.1, so now, not sure why but github shows those changed file too in the above "Files Changed" link, (showing them as green here even though they are already in the release/3.1 branch)

Here is a link for convenience which shows the actual diff:

https://github.com/mabe02/lanterna/compare/release/3.1...ginkoblongata:LANTERNA-491 https://github.com/mabe02/lanterna/compare/release/3.1...ginkoblongata:LANTERNA-491

[ginkoblongata]

testing with this:

$ mvn clean install ; java -cp "target/lanterna-3.1.0-SNAPSHOT-tests.jar:target/lanterna-3.1.0-SNAPSHOT.jar" -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 com.googlecode.lanterna.gui2.ImageComponentTest --mouse-move

[ginkoblongata]

This is a breaking change:

https://github.com/mabe02/lanterna/pull/492/commits/fc3c0e1dcbf50aca36fb501abcf99b69287411a5

Removed the goofy foot method setCaretPosition(int line, int column), replaced with setCaretPosition(TerminalPosition position).

It would be trivial to add it back if that is deemed desirable as part of this pull request.

[ginkoblongata]

I guess the activity around this pull request may be settling, I am not aware something further for it.

[avl42]

I couldn't follow this thread in detail, but the topic "scrolling" triggers my memory about one of my past contributions: TerminalScreen's "scrollLines" method, which makes scrolling-by-lines more efficient, if the scrolling area covers most of the terminal width. So, if the new ScrollArea covers more than 2/3s of terminal width, then using ScrollLines will reduce refresh-flicker noticeably.

If you do not already use it, then I'd strongly recommend doing so, otherwise, all the better :-)

[ginkoblongata]

If I understood, some terminals implement scrolling with some kind of control characters or something?

In those cases, do the scroll have to happen for the entire line? I see that things on the Lanterna side can scroll and be arbitrary width, but once they hit the terminal, do those scrollable Terminals support arbitrary regions of scrolling? Also, looks like the scrolling would be vertically but is there horizontally also? Thanks

[ginkoblongata]

testing with these:

$ mvn clean install ; java -cp "target/lanterna-3.1.0-SNAPSHOT-tests.jar:target/lanterna-3.1.0-SNAPSHOT.jar" -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 com.googlecode.lanterna.gui2.SplitPanelTest --mouse-move $ mvn clean install ; java -cp "target/lanterna-3.1.0-SNAPSHOT-tests.jar:target/lanterna-3.1.0-SNAPSHOT.jar" -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005 com.googlecode.lanterna.gui2.ThemeTest --mouse-move

[ginkoblongata]

Looks good to me in Xterm.

[avl42]

Yes, Terminals usually support an escape sequence, that will restrict scrolling to a given range of lines and other sequences to insert and delete lines. I do not know of any sequences to support horizontal scrolling. It's probably a much rarer usecase in practice than that of vertical scrolling of whole lines, anyway ;-)

Lanterna uses a back-buffer and a front-buffer to optimize screen redrawing: the back-buffer contains what should be on screen, and the front-buffer represents what lanterna thinks, that the terminal actually looks like.

If we now assume (for the sake of demonstration) that the terminal shows two panels side by side, where panel A shows lines of letters ("aaa...", "bbb...",...) and covers more than 3/4 of total width, whereas panel B shows lines of digits ("111...","222...",...) and let there also be window-borders that are the same char for the total height of the panels. Finally lets assume, that some user-interaction now causes panel A to scroll down by one line (in the sense of bringing into view the next line below current view - thus moving lines up).

So far, Lanterna would let the scrolled components draw themselves into the backbuffer, then determine lots of differences to the front-buffer, and redraw the whole screen.

Using "scrollLines", Lanterna would "know" that the application scrolled the lines covered by panel A, and during redraw it will: 1.) send the escapes to the terminal to perform the scrolling, 2.) scroll the front-buffer accordingly, 3) notice that the differences are only 1/4 of the screen, do an refreshByDelta for the stuff in panel B, which got previously scrolled along. (The borders themselves did almost not visibly change; only the bottom line of the borders will need to be redrawn.)

From this it becomes clear, that the scrollLines-approach is only helpful, if a certain percentage of the horizontal estate is actually being scrolled.

My implementation only covered Screen, but probably could be extended: We would need to extract ScrollHint-Management from TerminalScreen.scrollLines into a separate method, and for a Window or Component, implement scrollLines such, that if the element covers at least some threshold of horizontal estate on screen, then it would set scrollHints on the Screen (though without actually scrolling the screen!).

The result should be: if a sufficiently horizontally-large Component scrolls, then the Screen's ScrollHint shall be updated, such that the next refresh will be more efficient than current redraw-it-all.

PS: my scrollLines implementation was triggered by someone complaining that his application was too slow/flickery when scrolling.

On Mon, Jun 22, 2020 at 9:52 PM ginkoblongata notifications@github.com wrote:

If I understood, some terminals implement scrolling with some kind of control characters or something?

In those cases, do the scroll have to happen for the entire line? I see that things on the Lanterna side can scroll and be arbitrary width, but once they hit the terminal, do those scrollable Terminals support arbitrary regions of scrolling? Also, looks like the scrolling would be vertically but is there horizontally also? Thanks

[ginkoblongata]

Thanks for the explanation, good to know.

[ginkoblongata]

I think some layer in between the Component and the Screen can do some napkin math and determine on the fly if it would invoke the scrollLines() for the blit. At the Component level, I think it makes sense to render the entire component in the general case and some components may attempt to help with getting to that call quicker, but at the application development time because generally the Component may be arbitrarily positioned and occluded on screen at any particular moment.

[ginkoblongata]

Ok, looks like there is now still one thing undone with the pull request.

Now the popup menu windows are 'dismissable' when clicking outside of them, or that is what the user may be led to think, but really that is just an artifact of the clicking bringing up another window over top of the popup.

If the user then drags the window away, they can reveal the unspent popup menu.

I guess I will have to look into that as part of this pull request.

[ginkoblongata]

Ok, popups menu's now close when user clicks outside their bounds.

[mabe02]

Ok, so I've reviewed this again and the big issue now is that this PR contains multiple unrelated changes. Can you please rebase this on release/3.1 and only include the relevant changes for ScrollPanel? The other changes should go in as separate PRs, if you want to push them too.

[ginkoblongata]

I think it makes sense to break out some of the unrelated changes and make PR's for those and get those through the pipeline before revisiting this PR. Then this PR can be revisited after those are in release/3.1 and then this PR can rebase those back in and the diff will be smaller between this branch and release/3.1.

[ginkoblongata]

Just dropping a note here, haven't forgot about this, hopefully some more activity in a couple weeks.

[mabe02]

@ginkoblongata would you be opening a new PR or update this one?

[ginkoblongata]

I would like to keep this one open, and eventually have it accepted for merge. But I will make another 1 or 2 slices off I think with new PR.

[ginkoblongata]

Just dropping another note, I am still committed to this change. Other commitments are currently taking priority at this time still.

[scolastico]

Would be very interrested to see that feature.

[mabe02]

Stale