9front - general discussion about 9front
 help / color / mirror / Atom feed
From: istvan bak <bdhpfl@gmail.com>
To: 9front@9front.org
Subject: Re: [9front] focus bug in libdraw program
Date: Thu, 22 Apr 2021 15:26:17 +0000	[thread overview]
Message-ID: <CAO+DOcoJMtyGVvL70a0PmZYJP=rPHhrt0D0yhaevm50VzKrutA@mail.gmail.com> (raw)
In-Reply-To: <54D0DFB7C443777FB95F48DA583D5F84@gmail.com>

In case nobody touched this yet, here's my take on it.

two symptom notes:
1. if you print the whole Mouse struct you receive, when reproducing
the bug the timestamps are always the same. In fact the whole struct
value is always the same.
2. if you run the program in a subrio, and summon the focus-grabbing
window, and then don't give the focus back to the program, then
resizing the subrio also reproduces the bug. You don't need the
Hide/Unhide for it.

relevant routines:
/sys/src/libdraw/mouse.c:/^_ioproc/
/sys/src/cmd/rio/xfid.c:/^xfidread/
/sys/src/cmd/rio/wind.c:/^winctl/
/sys/src/cmd/rio/wind.c:/^wresize/

phil's program gets a resize from mctl->resizec, then a mouse message
from mctl->c. This is because _ioproc() reads from /dev/mouse a single
'r' message, which it splits into a send(mc->resizec), then a
send(mc->c). This is correct, I believe 'r' may hold a valid mouse
event (can't confirm/deny, haven't read all of rio, yet). xfidread()
checks w->resized and rewrites 'm' to 'r'. It gets the mouse message
from winctl(), which either gets it from the queue (<- only clicks go
here I think), or copies "the current mouse state" (<- mouse motion is
reported such, this is not a rare code path). Hide/Unhide is a resize
(see routines whide, wsendctlmesg, wctlmesg), wresize() sets
w->resized, and increments w->mouse.counter. The counter is a serial
number for mouse events, which winctl() looks at for changes, so
wresize() faked a mouse event to be delived in a mouse message.

the bug:
usually, the wresize() counter++ creates (in winctl()'s WMouseread
case) a mouse message holding the "current mouse state", and it's
usually to the effect of "i moved from current position to current
position at the exact same timestamp", which I guess is a no-op for
most client programs. The "current" state is the last state as when
the window had focus (or whatever). With the above program though, the
last state is "right click into the rectangle". So the resize
generates a bogus mouse event with the Mouse.buttons field set.

weakly proposed fix:
make the 'r' message be not a resize+mouse message, but just a resize
message with zeroed-out fields after it. libdraw (and some other
programs) will have to be tweaked, to not fall through from 'r' to
'm', but to break out instead. Thus, a resize won't generate a bogus
mouse event, be it no-op or otherwise. I do not yet see what problems
this may cause.

Programs that open /dev/mouse would have to be tweaked, I don't know
how many, a quick grep reveals a dozen. I considered making it an 'R'
message and keeping the old 'r' as well, but I think that causes bugs
and confusion. Also, the fix makes the file interface subtly different
from the bell labs version ☹. Arguably that deserves a bugfix too.

Someone who knows rio source better should chime in, please. If not, I
will return in a few weeks after having consumed it.

Example patch, feel free to make it less clumsy (and note, gmail will
fucking mangle the long lines):

diff -r e43de7ad21b2 sys/src/cmd/rio/dat.h
--- a/sys/src/cmd/rio/dat.h	Thu Apr 22 08:40:50 2021 +0200
+++ b/sys/src/cmd/rio/dat.h	Thu Apr 22 16:34:15 2021 +0200
@@ -154,6 +154,7 @@
 	 * Now they're always the same but the code doesn't assume so.
 	*/
 	Rectangle		screenr;	/* screen coordinates of window */
+	int			sendresized;
 	int			resized;
 	int			wctlready;
 	Rectangle		lastsr;
diff -r e43de7ad21b2 sys/src/cmd/rio/wind.c
--- a/sys/src/cmd/rio/wind.c	Thu Apr 22 08:40:50 2021 +0200
+++ b/sys/src/cmd/rio/wind.c	Thu Apr 22 16:34:15 2021 +0200
@@ -376,9 +376,8 @@
 	flushimage(display, 1);
 	wsetname(w);
 	w->topped = ++topped;
-	w->resized = TRUE;
+	w->sendresized = TRUE;
 	w->winnameread = FALSE;
-	w->mouse.counter++;
 	w->wctlready = 1;
 }

@@ -1549,7 +1548,7 @@
 		} else {
 			alts[WKbdread].op = (w->kbdopen && kbdqw != kbdqr) ?
 				CHANSND : CHANNOP;
-			alts[WMouseread].op = (w->mouseopen && w->mouse.counter !=
w->mouse.lastcounter) ?
+			alts[WMouseread].op = (w->mouseopen && w->mouse.counter !=
w->mouse.lastcounter || w->sendresized) ?
 				CHANSND : CHANNOP;
 			alts[WCwrite].op = w->scrolling || w->mouseopen || (w->qh <=
w->org+w->nchars) ?
 				CHANSND : CHANNOP;
@@ -1625,6 +1624,14 @@
 				wmousectl(w);
 			break;
 		case WMouseread:
+			if(w->sendresized){
+				w->sendresized = 0;
+				w->resized = 1;
+				memset(&m, 0, sizeof(Mousestate));
+				send(mrm.cm, &m.Mouse);
+				continue;
+			}
+
 			/* send a queued event or, if the queue is empty, the current state */
 			/* if the queue has filled, we discard all the events it contained. */
 			/* the intent is to discard frantic clicking by the user during
long latencies. */
diff -r e43de7ad21b2 sys/src/libdraw/mouse.c
--- a/sys/src/libdraw/mouse.c	Thu Apr 22 08:40:50 2021 +0200
+++ b/sys/src/libdraw/mouse.c	Thu Apr 22 16:34:15 2021 +0200
@@ -68,7 +68,7 @@
 			one = 1;
 			if(nbsend(mc->resizec, &one) < 0)
 				continue;
-			/* fall through */
+			break;
 		case 'm':
 			m.xy.x = atoi(buf+1+0*12);
 			m.xy.y = atoi(buf+1+1*12);

  reply	other threads:[~2021-04-22 21:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14  6:03 telephil9
2021-04-22 15:26 ` istvan bak [this message]
2021-04-24 11:17   ` cinap_lenrek
2021-04-25 17:25     ` istvan bak
2021-04-26  5:04     ` telephil9

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAO+DOcoJMtyGVvL70a0PmZYJP=rPHhrt0D0yhaevm50VzKrutA@mail.gmail.com' \
    --to=bdhpfl@gmail.com \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).