NetBSD Problem Report #41223

From jld@xlerb.net  Wed Apr 15 19:00:22 2009
Return-Path: <jld@xlerb.net>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 4E1C063B8A5
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 15 Apr 2009 19:00:22 +0000 (UTC)
Message-Id: <20090415174214.1FF8915088@planetarium.xlerb.net>
Date: Wed, 15 Apr 2009 13:42:14 -0400 (EDT)
From: jld@panix.com
Reply-To: jld@panix.com
To: gnats-bugs@gnats.NetBSD.org
Subject: libcurses: curs_set and move do not cause refresh on getch
X-Send-Pr-Version: 3.95

>Number:         41223
>Category:       lib
>Synopsis:       libcurses: curs_set and move do not cause refresh on getch
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    blymn
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 15 19:05:00 +0000 2009
>Closed-Date:    Wed Sep 26 19:42:07 +0000 2018
>Last-Modified:  Wed Sep 26 19:42:07 +0000 2018
>Originator:     Jed Davis
>Release:        NetBSD 5.0_RC2
>Organization:
>Environment:
System: NetBSD planetarium.xlerb.net 5.0_RC2 NetBSD 5.0_RC2 (PLANETAR64) #3: Tue Mar 3 21:40:33 EST 2009 jld@city-of-dreadful-night.xlerb.net:/usr/local/src/nb-5/sys/arch/amd64/compile/PLANETAR64 amd64
Architecture: x86_64
Machine: amd64
>Description:

If one of the getch() family of functions is called, and the only
actions since the last refresh() on that window are applications of
curs_set() or move(), then the getch() will not perform the implicit
refresh() that seems to be called for by the documentation and the
standard.

>How-To-Repeat:
I've reproduced this bug on -current and -4 as well as -5.

#include <curses.h>
int main()
{ initscr(); for (;;) { curs_set(0); refresh(); curs_set(1); getch(); } }

#include <curses.h>
int main()
{ initscr(); for (;;) { move(10, 10); refresh(); move(0, 0); getch(); } }

Compare the behavior to that of other curses implementations (e.g.,
ncurses), or the behavior with an explicit refresh() before the getch().

These testcases are reduced from the behavior of the roguelike game
Dungeon Crawl Stone Soup (http://crawl-ref.sf.net/); with NetBSD curses,
the cursor is usually invisible when waiting for input, including the
selection of a location on the screen with the cursor.  The bug with
move is also visible at one point.

>Fix:

The problem seems to be that wgetch (and wget_wch) call wrefresh(win)
only if is_wintouched(win) is true; however, is_wintouched (which is
just the disjunction over all lines of is_linetouched) will be made true
only if characters are actually written to the screen.

Note that while curs_set() writes termcap codes immediately instead of
deferring work to update time, it does so via __cputchar, which is a
stdio buffered write that isn't flushed until the end of do_update.

One fix would be to simply remove the is_wintouched() tests; given
that that should amount to one extra refresh per keypress, and only if
the application is doing no screen output (including echo, I think) in
response, the performance overhead should be negligible.  It would also
be possible to track the cursor dirtiness explicitly, but I'm not sure
it's worth it.

>Release-Note:

>Audit-Trail:
From: Jed Davis <jld@panix.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41223: libcurses: curs_set and move do not cause refresh
	on getch
Date: Tue, 21 Apr 2009 01:22:10 -0400

 I've prepared a patch to make the change I proposed (of removing the
 is_wintouched check), and in minimal testing it appears to work.  See
 http://www.NetBSD.org/~jld/nbcurses-getch-refresh.diff , or below:

 Index: lib/libcurses/getch.c
 ===================================================================
 RCS file: /bag/nb/repo/src/lib/libcurses/getch.c,v
 retrieving revision 1.51.8.1
 diff -u -p -r1.51.8.1 getch.c
 --- lib/libcurses/getch.c	18 Feb 2009 01:13:54 -0000	1.51.8.1
 +++ lib/libcurses/getch.c	21 Apr 2009 04:46:59 -0000
 @@ -804,8 +804,7 @@ wgetch(WINDOW *win)
  	    && __echoit)
  		return (ERR);

 -	if (is_wintouched(win))
 -		wrefresh(win);
 +	wrefresh(win);
  #ifdef DEBUG
  	__CTRACE(__CTRACE_INPUT, "wgetch: __echoit = %d, "
  	    "__rawmode = %d, __nl = %d, flags = %#.4x, delay = %d\n",
 Index: lib/libcurses/get_wch.c
 ===================================================================
 RCS file: /bag/nb/repo/src/lib/libcurses/get_wch.c,v
 retrieving revision 1.6
 diff -u -p -r1.6 get_wch.c
 --- lib/libcurses/get_wch.c	14 Apr 2008 20:33:59 -0000	1.6
 +++ lib/libcurses/get_wch.c	21 Apr 2009 04:47:08 -0000
 @@ -516,8 +516,7 @@ wget_wch(WINDOW *win, wint_t *ch)
  			&& __echoit)
  		return (ERR);

 -	if (is_wintouched(win))
 -		wrefresh(win);
 +	wrefresh(win);
  #ifdef DEBUG
  	__CTRACE(__CTRACE_INPUT, "wget_wch: __echoit = %d, "
  	    "__rawmode = %d, __nl = %d, flags = %#.4x\n",

From: Brett Lymn <blymn@baesystems.com.au>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
        jld@panix.com
Subject: Re: lib/41223: libcurses: curs_set and move do not cause refresh on getch
Date: Tue, 21 Apr 2009 22:12:04 +0930

 On Tue, Apr 21, 2009 at 05:25:02AM +0000, Jed Davis wrote:
 > 
 >  I've prepared a patch to make the change I proposed (of removing the
 >  is_wintouched check), and in minimal testing it appears to work.  See
 >  http://www.NetBSD.org/~jld/nbcurses-getch-refresh.diff , or below:
 >

 I do not think this is the correct thing to do.  It is unacceptable to
 force a refresh of the window for every character as this may mean, in
 some very dumb terminal cases, a redraw of the screen.

 SUS v2 is not clear on the expected behaviour and I, personally, don't
 hold a certain other curses package as being authorative on curses
 behaviour.

 A peek at the Open Solaris curses code shows they perform a fflush on
 the output file descriptor after the tput - I think that would be a
 better solution.

 -- 
 Brett Lymn
 "Warning:
 The information contained in this email and any attached files is
 confidential to BAE Systems Australia. If you are not the intended
 recipient, any use, disclosure or copying of this email or any
 attachments is expressly prohibited.  If you have received this email
 in error, please notify us immediately. VIRUS: Every care has been
 taken to ensure this email and its attachments are virus free,
 however, any loss or damage incurred in using this email is not the
 sender's responsibility.  It is your responsibility to ensure virus
 checks are completed before installing any data sent in this email to
 your computer."


From: Brett Lymn <blymn@baesystems.com.au>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: lib/41223: libcurses: curs_set and move do not cause refresh on getch
Date: Tue, 21 Apr 2009 23:16:55 +0930

 Try this patch to see if it makes a difference:

 Index: curs_set.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libcurses/curs_set.c,v
 retrieving revision 1.8
 diff -u -r1.8 curs_set.c
 --- curs_set.c	21 Jan 2007 13:25:36 -0000	1.8
 +++ curs_set.c	21 Apr 2009 13:44:47 -0000
 @@ -59,6 +59,7 @@
  #endif
  				_cursesi_screen->old_mode = 0;
  				tputs(__tc_vi, 0, __cputchar);
 +				fflush(_cursesi_screen->outfd);
  				return old_one;
  			}
  			break;
 @@ -70,6 +71,7 @@
  #endif
  				_cursesi_screen->old_mode = 1;
  				tputs(__tc_ve, 0, __cputchar);
 +				fflush(_cursesi_screen->outfd);
  				return old_one;
  			}
  			break;
 @@ -82,6 +84,7 @@
  #endif
  				_cursesi_screen->old_mode = 2;
  				tputs(__tc_vs, 0, __cputchar);
 +				fflush(_cursesi_screen->outfd);
  				return old_one;
  			}
  			break;

 -- 
 Brett Lymn
 "Warning:
 The information contained in this email and any attached files is
 confidential to BAE Systems Australia. If you are not the intended
 recipient, any use, disclosure or copying of this email or any
 attachments is expressly prohibited.  If you have received this email
 in error, please notify us immediately. VIRUS: Every care has been
 taken to ensure this email and its attachments are virus free,
 however, any loss or damage incurred in using this email is not the
 sender's responsibility.  It is your responsibility to ensure virus
 checks are completed before installing any data sent in this email to
 your computer."


Responsible-Changed-From-To: lib-bug-people->blymn
Responsible-Changed-By: blymn@NetBSD.org
Responsible-Changed-When: Wed, 22 Apr 2009 13:55:18 +0000
Responsible-Changed-Why:
I have already generated a fix for this so I may as well have the pr


State-Changed-From-To: open->feedback
State-Changed-By: blymn@NetBSD.org
State-Changed-When: Wed, 22 Apr 2009 13:55:18 +0000
State-Changed-Why:
Awaiting feedback for patch provided.


From: Jed Davis <jld@panix.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: lib/41223: libcurses: curs_set and move do not cause refresh
	on getch
Date: Thu, 7 May 2009 22:18:39 -0400

 On Tue, Apr 21, 2009 at 12:45:02PM +0000, Brett Lymn wrote:
 >  I do not think this is the correct thing to do.  It is unacceptable to
 >  force a refresh of the window for every character as this may mean, in
 >  some very dumb terminal cases, a redraw of the screen.

 Oh; I'd assumed that libcurses would be able to avoid unnecessary
 redrawing even on dumber terminals.  

 I'd also assumed that most getch()es would require a refresh anyway, but
 now that I've done a bit of testing with an instrumented libcurses I see
 that this isn't necessarily so.

 >  A peek at the Open Solaris curses code shows they perform a fflush on
 >  the output file descriptor after the tput - I think that would be a
 >  better solution.

 The other thing that OpenSolaris curses does is for wmove to set a bit
 in the per-WINDOW flags field, which causes a refresh on the next getch.
 ( http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libcurses/screen/wmove.c#72 )
 ( http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libcurses/screen/wgetch.c#84 )

 In the application where I noticed this issue, just making curs_set do
 the fflush and not also taking care of the wmove causes the cursor to be
 visible and in the wrong place.

 --Jed

State-Changed-From-To: feedback->open
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 03 May 2010 03:25:42 +0000
State-Changed-Why:
Feedback was received a year ago. (doh)


State-Changed-From-To: open->closed
State-Changed-By: kamil@NetBSD.org
State-Changed-When: Wed, 26 Sep 2018 21:42:07 +0200
State-Changed-Why:
Fixed in HEAD in src/lib/libcurses/move.c r.1.19 via a duplicate/similar bug in lib/53617


>Unformatted:

Home	PR Database Search