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 356552 - cursor timeout runs all the time
cursor timeout runs all the time
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.14.x
Other All
: Normal normal
: ---
Assigned To: VTE Maintainers
VTE Maintainers
: 354021 354801 (view as bug list)
Depends on:
Blocks: 356586
 
 
Reported: 2006-09-18 15:07 UTC by Allison Karlitskaya (desrt)
Modified: 2006-12-05 22:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (12.53 KB, patch)
2006-09-18 15:54 UTC, Allison Karlitskaya (desrt)
none Details | Review
update (13.60 KB, patch)
2006-09-18 17:06 UTC, Allison Karlitskaya (desrt)
none Details | Review
new patch (10.73 KB, patch)
2006-09-18 18:44 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2006-09-18 15:07:54 UTC
the cursor timeout should be disabled when:

1) blinking cursor is not enabled
2) the window is not focused.
Comment 1 Allison Karlitskaya (desrt) 2006-09-18 15:54:11 UTC
Created attachment 72986 [details] [review]
patch

the patch fixes the problem and generally reworks the cursor blinking stuff to be way less complicated.

it also cleans up some warnings that kept bugging me while i was working.
Comment 2 Allison Karlitskaya (desrt) 2006-09-18 16:03:15 UTC
*** Bug 354801 has been marked as a duplicate of this bug. ***
Comment 3 Allison Karlitskaya (desrt) 2006-09-18 17:06:11 UTC
Created attachment 72991 [details] [review]
update

this fixes a problem with the last patch where the cursor would blink irrespective of keyboard input (because the last_keypress_event variable was being ignored).

this patch nukes last_keypress_event entirely and just reschedules the timer.

nice side effect: the cursor will now blink at exactly the right time after the user stops typing -- not just whenever the timer happened to run next.
Comment 4 Behdad Esfahbod 2006-09-18 17:19:20 UTC
Thanks Ryan.  It really helps in the future if you separate warning fixes from other changes.

Also, while you're at it, do you mind implementing support for blink timeout?  Can't find the bug, but you know which one I mean.
Comment 5 Allison Karlitskaya (desrt) 2006-09-18 18:44:42 UTC
Created attachment 72996 [details] [review]
new patch

added the cursor timeout stuff that's in gtk HEAD now -- graceful fallback for old gtk versions.

as requested, fix for the warnings moved to bug #356602
Comment 6 Behdad Esfahbod 2006-09-18 19:34:23 UTC
*** Bug 354021 has been marked as a duplicate of this bug. ***
Comment 7 Behdad Esfahbod 2006-09-18 19:38:02 UTC
Humm, is gint64 really necessary?  I know the gtk property uses G_MAXINT to mean "blink forever".
Comment 8 Allison Karlitskaya (desrt) 2006-09-18 19:42:21 UTC
The timeout runs once every half second (or perhaps other strange amounts of time as specified by Xsettings).  This means that we can't count the number of seconds elapsed (without using a float) so instead I count the number of milliseconds.

If I put milliseconds in a 32bit signed int, it would wrap after 24 days.  This means that if someone had a timeout of (say) 30 days then the cursor would never stop blinking.
Comment 9 Behdad Esfahbod 2006-09-18 20:00:00 UTC
K.

I'm also wondering: gtk+ blinks cursor 2 on 1 off.  vte does 1 on 1 off.  Should we change?
Comment 10 Allison Karlitskaya (desrt) 2006-09-18 20:08:30 UTC
Linux console does 1/1, which is probably closest to the feel we're trying to emulate.  I just tried 2/1 in gnome-terminal and I'm not really a fan of it.

fwiw, ephy (or at least firefox) does 1/1 in text entry fields on, for example, bugzilla :)
Comment 11 Matthias Clasen 2006-09-20 04:50:12 UTC
Ryan, I honestly think that a blink timeout of 24 days is not something to worry about...
Comment 12 Allison Karlitskaya (desrt) 2006-09-20 12:32:08 UTC
Totally reasonable!  I'd totally set a blink timeout of 24 days.  Think about all of the power you could save running in the 25th day on your laptop's battery!!

But seriously, *shrug*  I just did it because it was a case that might hypothetically occur and it was more correct to acknowledge the problem than not to.  I'll commit the patch without it if a 64bit int is that unfashionable. :)

note:  A legit case is if the timeout is INT_MAX.  If we use a signed int then the largest value it will take on is INT_MAX.  When we divide this by 1000 it will be less than the timeout of INT_MAX.  Once it wraps to having a negative value it will continue to be less than INT_MAX.  The timer will keep running forever.  This is what we want.

The only (very much edge) case left is when the user changes their cursor blink timeout from INT_MAX to [something] after they've had terminals running for more than 24 days.  In this case, their timeout will be negative and it will take another 24 days to catch up to their new timeout value.  We could use unsigned to correct for this and buy ourselves another 24 days for the normal case while we're at it :)
Comment 13 Matthias Clasen 2006-12-05 04:41:21 UTC
Behdad, can we get this committed ?

gnome-terminal is one of the major offenders in terms of context switches on my desktop, due to this.
Comment 14 Behdad Esfahbod 2006-12-05 04:57:59 UTC
I'm not sure the cursor timeout is enough.  I /think/ vte has a coalesce timeout running permanently, waiting for input, but I may be wrong.  I'll look into it tomorrow.
Comment 15 Allison Karlitskaya (desrt) 2006-12-05 05:35:47 UTC
iirc this is all you need.

The coalesce timer is good enough to cancel itself.  When I did my testing with this all was entirely quiet on my desktop.
Comment 16 Behdad Esfahbod 2006-12-05 22:29:42 UTC
Committed:

2006-12-05  Ryan Lortie  <desrt@desrt.ca>

        Bug 356552 – cursor timeout runs all the time

        * src/vte-private.h: Rework how cursor blinking works.
        * src/vte.c: Rework how cursor blinking works.  Only register the
        cursor blink callback when cursor blink is enabled and the window
        is focused.