After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 709089 - Scrollbar flashes up when shrinking window with alternate screen
Scrollbar flashes up when shrinking window with alternate screen
Status: RESOLVED OBSOLETE
Product: vte
Classification: Core
Component: general
0.34.x
Other Linux
: Normal minor
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-30 12:41 UTC by Egmont Koblinger
Modified: 2021-06-10 14:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
refactoring (8.75 KB, patch)
2013-09-30 21:02 UTC, Egmont Koblinger
none Details | Review
proof of concept flash (485 bytes, patch)
2013-09-30 22:14 UTC, Egmont Koblinger
none Details | Review
refactoring (10.62 KB, patch)
2014-05-22 01:19 UTC, Egmont Koblinger
none Details | Review
proof of concept patch (592 bytes, patch)
2014-05-22 01:20 UTC, Egmont Koblinger
none Details | Review
Fix v2 (797 bytes, patch)
2015-11-08 18:32 UTC, Egmont Koblinger
none Details | Review
Fix v3 (610 bytes, patch)
2017-09-18 20:27 UTC, Egmont Koblinger
none Details | Review

Description Egmont Koblinger 2013-09-30 12:41:46 UTC
Switch to alternate screen, resize the window to contain fewer lines. Notice that the scrollbar(*) flashes up for a short time.

(*) I'm using Ubuntu 13.04's default desktop with its overlay scrollbar that's invisible when it can't scroll anything, that is, it should always be invisible with the alternate screen. Probably the glitch is less noticable with a regular scrollbar.

In vte_terminal_size_allocate() first we call vte_terminal_set_size() which figures out that a scrollbar is required now and schedules it to be displayed. It is followed by vte_terminal_set_scrollback_lines() which truncates the ring so that scrollbar is no longer required.

Not sure how it behaves / should behave when normal screen is on with scrolling restricted.

(I was about to add an assertion in vte_terminal_screen_set_size() that with the alternate screen we need to enter the "everything fits without scrollbars" branch, but right now this assertion doesn't hold.)

Technical thoughts:

I don't like the vte_terminal_queue_adjustment_value_changed() method. It makes it required that the screen's scroll_delta is set via a method and not directly, as opposed to all the other fields. Except when the screen is not the active one, see the ugly "if" branch at the end of vte_terminal_screen_set_size() (bug 415277's fix). It requires an ugly hack in bug 676075's fix. The design doesn't consider that the scrollbar is per-terminal while scroll_delta is per-screen.

The code cleanup I have in my mind is: Just set screen's scroll_delta directly. Have a copy in VteTermial which is for the actually displayed value. Have a method that updates the terminal's scroll_delta based on the active screen's scroll_delta and schedules a visual update if the value actually changed.
Comment 1 Egmont Koblinger 2013-09-30 21:02:27 UTC
Created attachment 256137 [details] [review]
refactoring

This is the refactoring I had in my mind. I'm afraid it doesn't get us any closer to actually fix this bug...
Comment 2 Christian Persch 2013-09-30 21:13:33 UTC
(In reply to comment #0)
> In vte_terminal_size_allocate() first we call vte_terminal_set_size() which
> figures out that a scrollbar is required now and schedules it to be displayed.
> It is followed by vte_terminal_set_scrollback_lines() which truncates the ring
> so that scrollbar is no longer required.

Acutually, no; vte doesn't do *anything* with scrollbars. g-t adds it, and it only shows it always or never, the 'only when required' mode isn't implemented.

So I rather suspect this to be a bug in ubuntu's scrollbar overlay module.
Comment 3 Egmont Koblinger 2013-09-30 21:24:37 UTC
The behavior is also visible with regular scrollbar. And both vteapp and g-t behave this way. I don't know if it's vte itself, or both vteapp and g-t that displays the scrollbar. But I think that for a short time during resizing, vte gives false instructions wrt the scrollbar. Maybe the parameters (upper, lower, adj value, page size etc.) are not updated atomically and a gtk display update kicks in. Unfortunately we're entering the maze of gtk world where I hardly have any experience.
Comment 4 Egmont Koblinger 2013-09-30 22:14:34 UTC
Created attachment 256141 [details] [review]
proof of concept flash

The bug is clearly observable if you significantly increase VTE_UPDATE_TIMEOUT, the scrollbar is visible for the whole duration of this timeout.

The adjustment's page size is updated immediately, whereas lower and upper are only updated after the timeout.

My quick prototype patch updates the upper bound immediately too.
Comment 5 Egmont Koblinger 2013-09-30 22:15:41 UTC
proof of concept *patch* :D
Comment 6 Egmont Koblinger 2013-10-02 12:14:24 UTC
It's more complex than it looks... for debugging, raise VTE_UPDATE_TIMEOUT to 1000 or so.

Notice than upon starting vteapp, the scrollbar (both standard and overlay) is drawn incorrectly for this amount of time, then it gets fixed. This is because the page size is initialized immediately, but the bounds are only initialized after the first timeout.

My first attempt went in the direction that _vte_terminal_adjust_adjustments_full() should not only update the page size synchronously, but also the bounds, we shouldn't wait for a timer in this method. The result is: (i) standard scrollbar is fine, but the overlay scrollbar shows up in the full height of the window until the bounds change (e.g. press enter at the prompt).

There's another issue with the overlay scrollbar versus gnome-terminal, and it's apparently not vte's or g-t's fault:
(ii) https://bugs.launchpad.net/ubuntu/+source/gnome-terminal/+bug/827380

I suspect that the overlay scrollbar incorrectly draws itself initially, when it hooks up to an already existing GtkAdjustment. At least, this would explain both symptoms (i) and (ii), and would explain why in current vte accidentally setting the bounds only after a short timeout accidentally repairs the initial look of the scrollbar (i.e. does not show).

Also, as for a bit of cleanup in vte: the doc of _vte_terminal_adjust_adjustments_full() lists a couple of cases when to call this method, none of them is valid, and misses the only valid one: this method should be called only when the terminal height changes. Maybe it could even be inlined in vte_terminal_set_size().

I'd say we can either go for an ugly workaround (such as my previous patch), or for a proper fix and code cleanup (adjusting everything synchronously when the terminal is resized). The proper fix is blocked by the overlay scrollbar bug (if we care about Ubuntu's private actions).

(Note: use LIBOVERLAY_SCROLLBAR=0 to disable overlay scrollbar on Ubuntu.)
Comment 7 Christian Persch 2014-04-22 16:28:36 UTC
BTW, trying just now, can't see any flashing scrollbars.
Comment 8 Egmont Koblinger 2014-04-22 16:56:36 UTC
Have you tried increasing the update timeout to 1 second or so, as per comment 4?
Comment 9 Christian Persch 2014-04-22 17:08:16 UTC
Yes; it made no difference. Using gnome-shell 3.8 (fedora 19), maybe the WM is relevant here.
Comment 10 Egmont Koblinger 2014-04-22 17:18:53 UTC
I don't think it does, it really shouldn't. Maybe the GTK+ version matters, but I don't think that either.

Double check that you are on alternate screen (e.g. run "mc"). The scrollbar is visible but "unusable" (is of full height) normally. After shrinking the terminal height, for the duration of the update timeout it appears as a "usable" scrollbar, the actual bar is shorter than its slot and is grabbable.

Note that the scrollbar is also shorter initially, when the terminal is opened.

It was quite clear when I looked at this issue that parts of the scrollbar properties were updated immediately, and other parts were updated asynchronously after the timeout. I can't recall the exact details, I would need to dig in again.

I'm still on the 0.36/3.12 branch, but I don't think there were any relevant changes in master.
Comment 11 Christian Persch 2014-04-22 17:34:58 UTC
I only see that the scrollbar isn't full until the timeout has run, but that's expected (and with the usual short timeout, not noticeable). No flashing.

I'm not seeing it in vte 0.34+gtk3.8 either, so it's not a gtk change that's fixed it.
Comment 12 Egmont Koblinger 2014-04-22 17:40:45 UTC
Okay, we're on the same page there.

If you look at the bottom of the scrollbar, you should see a flicker of colors (scrollbar vs its slot). I'm using Ubuntu's overlay scrollbar which only shows up when it's actually usable, so it flashes up for this duration. It's caused by the same underlying issue.

We shouldn't give back control to GTK with inconsistent scrollbar properties. Currently, during this timeout, GTK updates the scrollbar's look with some of its properties (page size IIRC?) already updated, some others (bar height IIRC?) not yet updated.

I think all the scrollbar properties should immediately (synchronously) be updated on window resize.
Comment 13 Egmont Koblinger 2014-05-22 01:19:51 UTC
Created attachment 276964 [details] [review]
refactoring

(updated to current git)
Comment 14 Egmont Koblinger 2014-05-22 01:20:27 UTC
Created attachment 276965 [details] [review]
proof of concept patch

(updated to current git)
Comment 15 Egmont Koblinger 2015-09-26 01:23:37 UTC
As of Ubuntu Wily, overlay scrollbar is no more. So I guess fixing this bug is no longer blocked by other issues.
Comment 16 Christian Persch 2015-09-26 17:39:57 UTC
Hmm. Maybe we should use gdk frame clock, and make sure the adjustment is correct before each frame?
Comment 17 Egmont Koblinger 2015-11-08 18:32:39 UTC
Created attachment 315089 [details] [review]
Fix v2

Can't remember what/why I did in my previous changes, so starting from scratch :) No clue how gdk frames work.

Here's a one-liner fix. After a resize don't just queue an adjustment update but also execute it immediately. (Alas there's no separate method for this and I don't feel like refactoring.)
Comment 18 Egmont Koblinger 2016-03-07 09:55:28 UTC
The issue is even more visible if you stare at the scrollbar while keep opening new tabs.

Christian, any objections against committing "Fix v2" to master (and perhaps even to 0-44)?
Comment 19 Christian Persch 2016-03-07 12:31:44 UTC
I don't think that's the right patch. Ie what happens if we do set_size during processing (via the size change signals), will the immediate adjust cause problems?

I think the right fix would be to run the actual update of the adjustment right before the next tick of the frame clock.
Comment 20 Egmont Koblinger 2016-03-07 21:10:53 UTC
As for functional correctness, I think a delayed resize is much more dangerous than an immediate one. We'd need to come up with some testing for the use case you have in mind (I'm not sure I fully get that.)

Using my standard "time cat largefile" I couldn't measure any performance regression with an infinite scrollback, even though the adjustment is updated way more often (if I'm not mistaken). Is there any chance that the relevant Gtk+ method is clever enough not to do anything immediately other than some trivial bookkeeping, and do the heavy work on the next frame clock? In that case there'd be no point in us repeating this.

(Anyway, there's plenty of time until 0.46. Let's forget 0.44 then, I don't want to rush here.)
Comment 21 Egmont Koblinger 2017-09-18 20:27:29 UTC
Created attachment 360013 [details] [review]
Fix v3

Updated to the current codebase.
Comment 22 GNOME Infrastructure Team 2021-06-10 14:44:17 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vte/-/issues/2024.