9front - general discussion about 9front
 help / color / mirror / Atom feed
* [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
@ 2021-04-10 15:39 james palmer
       [not found] ` <b0646909-d59a-4423-8467-32c4c96ef5d3@www.fastmail.com>
  2021-04-11 15:39 ` [9front] " james palmer
  0 siblings, 2 replies; 13+ messages in thread
From: james palmer @ 2021-04-10 15:39 UTC (permalink / raw)
  To: 9front mailing list

[-- Attachment #1: Type: text/plain, Size: 270 bytes --]

attached is a patch for my rewrite of aux/wacom.
it prevents the "no concurrent reads please" error when moving the pen too fast,
as well as rotating the pen input to match the screen tilt.
i have kept the same format for /dev/tablet to keep aux/tablet working.

- james

[-- Attachment #2.1: Type: text/plain, Size: 329 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Disposition: attachment;filename="wacom.diff"
	Content-Type: text/x-patch; name="wacom.diff"
	Content-Transfer-Encoding: BASE64

[-- Attachment #2.2: wacom.diff.suspect --]
[-- Type: application/octet-stream, Size: 10675 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [9front] Re: [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
       [not found] ` <b0646909-d59a-4423-8467-32c4c96ef5d3@www.fastmail.com>
@ 2021-04-10 16:49   ` james palmer
  0 siblings, 0 replies; 13+ messages in thread
From: james palmer @ 2021-04-10 16:49 UTC (permalink / raw)
  To: 9front mailing list

[-- Attachment #1: Type: text/plain, Size: 196 bytes --]

ok, i promise this is the last bugfix.
now handling 9p requests in their own proc.
the problem was the read from aux/tablet was blocking,
preventing further requests from being answered.

- james

[-- Attachment #2.1: Type: text/plain, Size: 329 bytes --]

from postmaster@1ess:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Disposition: attachment;filename="wacom.diff"
	Content-Type: text/x-patch; name="wacom.diff"
	Content-Transfer-Encoding: BASE64

[-- Attachment #2.2: wacom.diff.suspect --]
[-- Type: application/octet-stream, Size: 10856 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-10 15:39 [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt james palmer
       [not found] ` <b0646909-d59a-4423-8467-32c4c96ef5d3@www.fastmail.com>
@ 2021-04-11 15:39 ` james palmer
  2021-04-12  7:37   ` hiro
  2021-04-12  8:12   ` cinap_lenrek
  1 sibling, 2 replies; 13+ messages in thread
From: james palmer @ 2021-04-11 15:39 UTC (permalink / raw)
  To: 9front mailing list

seems like the updated version of this got lost.
i fixed walks of /dev blocking by launching a new proc for each 9p request.
(i should probably convert this to use a Reqqueue, that will come soon)
patch is below because the ml keeps marking attached diffs as suspicious

- james

# HG changeset patch
# User foura <james@biobuf.link>
# Date 1618069652 -3600
# Node ID 0ce7cfc958df8ae0d2ff641f15251600795a48b5
# Parent  936eda80a52f65994713fc017d26bec3f6d72c8c
aux/wacom: rewrite to use multiple procs and support screen tilt.

diff -r 936eda80a52f -r 0ce7cfc958df sys/src/cmd/aux/wacom.c
--- a/sys/src/cmd/aux/wacom.c	Sat Apr 10 15:01:09 2021 +0200
+++ b/sys/src/cmd/aux/wacom.c	Sat Apr 10 16:47:32 2021 +0100
@@ -1,352 +1,335 @@
 #include <u.h>
 #include <libc.h>
+#include <bio.h>
+#include <thread.h>
 #include <fcall.h>
-#include <thread.h>
 #include <9p.h>
 
-typedef struct Tablet Tablet;
-typedef struct Message Message;
-typedef struct QItem QItem;
-typedef struct Queue Queue;
-typedef struct Reader Reader;
+enum {
+	None,
+	Left,
+	Right,
+	Inverted,
 
-
-enum { MAX = 1000 };
-
-struct Tablet {
-	int ser;
-	int xmax, ymax, pmax, version;
-	int sx, sy;
+	MAX = 1000,
+	STACK = 2048,
 };
 
-struct Message {
-	Ref;
-	int b, x, y, p;
-	char *msg;
+typedef struct Tablet Tablet;
+struct Tablet {
+
+	int fd;
+	char dev[64];
+	int version;
+
+	int x, y, b, p;
+	int xmax, ymax, pmax;
+
+	int sx, sy, srot;
 };
 
-Tablet*
-newtablet(char* dev)
+void
+threadfatal(char *fmt, ...)
 {
-	int serctl;
-	char* ctl;
-	Tablet* t;
+	va_list arg;
+	char buf[1024];
+	
+	va_start(arg, fmt);
+	vseprint(buf, buf+sizeof(buf), fmt, arg);
+	va_end(arg);
 
-	ctl = smprint("%sctl", dev);
-	t = calloc(sizeof(Tablet), 1);
-	t->ser = open(dev, ORDWR);
-	if(t->ser < 0) {
-		free(t);
-		return 0;
-	}
-	serctl = open(ctl, OWRITE);
+	if(argv0)
+		fprint(2, "%s: %s\n", argv0, buf);
+	else
+		fprint(2, "%s: %s\n", argv0, buf);
+
+	threadexitsall(buf);
+}
+
+void
+setuptablet(Tablet *t)
+{
+	int ctlfd;
+	char *ctl;
+	
+	ctl = smprint("%sctl", t->dev);
+	t->fd = open(t->dev, ORDWR);
+	if(t->fd < 0)
+		threadfatal("%r", t->dev);
+	
+	ctlfd = open(ctl, OWRITE);
+	if(ctlfd < 0)
+		threadfatal("%r", ctl);
 	free(ctl);
-	if(serctl < 0) {
-		free(t);
-		close(t->ser);
-		return 0;
-	}
-	if(fprint(serctl, "b19200\n") < 0) {
-		free(t);
-		close(t->ser);
-		close(serctl);
-		return 0;
-	}
-	close(serctl);
-	return t;
+
+	if(fprint(ctlfd, "b19200\n") < 0)
+		threadfatal("set baud rate: %r");
 }
 
 int
-query(Tablet* t)
+query(Tablet *t)
 {
 	uchar buf[11];
 
-	if(write(t->ser, "&0*", 3) < 3) return -1;
+	if(write(t->fd, "&0*", 3) < 3) return 0;
 	do {
-		if(read(t->ser, buf, 1) < 1) return -1;
+		if(read(t->fd, buf, 1) < 1) return 0;
 	} while(buf[0] != 0xC0);
-	if(readn(t->ser, buf+1, 10) < 10) return -1;
+
+	if(readn(t->fd, buf+1, 10) < 10) return 0;
+
 	t->xmax = (buf[1] << 9) | (buf[2] << 2) | ((buf[6] >> 5) & 3);
 	t->ymax = (buf[3] << 9) | (buf[4] << 2) | ((buf[6] >> 3) & 3);
 	t->pmax = buf[5] | (buf[6] & 7);
 	t->version = (buf[9] << 7) | buf[10];
-	if(write(t->ser, "1", 1) < 1) return -1;
-	return 0;
+
+	if(write(t->fd, "1", 1) < 1) return 0;
+	return 1;
 }
 
 int
-screensize(Tablet* t)
+findheader(Tablet *t)
+{
+	uchar c;
+
+	do {
+		if(read(t->fd, &c, 1) < 1) return -1;
+	} while((c & 0x80) == 0);
+
+	return c;
+}
+
+void
+readpacket(Tablet *t)
+{
+	uchar buf[9];
+	int head;
+
+	head = findheader(t);
+	if(head < 0) threadfatal("%r");
+	if(readn(t->fd, buf, 9) < 9) threadfatal("%r");
+
+	t->b = head & 7;
+	t->x = (buf[0] << 9) | (buf[1] << 2) | ((buf[5] >> 5) & 3);
+	t->y = (buf[2] << 9) | (buf[3] << 2) | ((buf[5] >> 3) & 3);
+	t->p = ((buf[5] & 7) << 7) | buf[4];
+}
+
+void
+screensize(Tablet *t)
 {
 	int fd;
 	char buf[189], buf2[12], *p;
-	
+
 	fd = open("/dev/draw/new", OREAD);
-	if(fd < 0) return -1;
+	if(fd < 0) threadfatal("%r");
 	read(fd, buf, 189);
+
 	memcpy(buf2, buf + 72, 11);
 	buf2[11] = 0;
 	for(p = buf2; *p == ' '; p++);
 	t->sx = atoi(p);
+
 	memcpy(buf2, buf + 84, 11);
 	for(p = buf2; *p == ' '; p++);
 	t->sy = atoi(p);
+
 	if(t->sx == 0 || t->sy == 0) {
-		close(fd);
-		werrstr("invalid resolution read from /dev/draw/new");
-		return -1;
+		threadfatal("invalid resolution read from /dev/draw/new");
 	}
-	
+
 	close(fd);
-	return 0;
-}
-
-int
-findheader(Tablet* t)
-{
-	uchar c;
-	
-	do {
-		if(read(t->ser, &c, 1) < 1) return -1;
-	} while((c & 0x80) == 0);
-	return c;
-}
-
-Message*
-readpacket(Tablet* t)
-{
-	uchar buf[9];
-	int head;
-	Message *m;
-
-	head = findheader(t);
-	if(head < 0) return 0;
-	if(readn(t->ser, buf, 9) < 9) return 0;
-	
-	m = calloc(sizeof(Message), 1);
-	incref(m);
-	
-	m->b = head & 7;
-	m->x = (buf[0] << 9) | (buf[1] << 2) | ((buf[5] >> 5) & 3);
-	m->y = (buf[2] << 9) | (buf[3] << 2) | ((buf[5] >> 3) & 3);
-	m->p = ((buf[5] & 7) << 7) | buf[4];
-	
-	m->p *= MAX;
-	m->p /= t->pmax;
-	m->x *= t->sx;
-	m->x /= t->xmax;
-	m->y *= t->sy;
-	m->y /= t->ymax;
-	
-	m->msg = smprint("m %d %d %d %d\n", m->x, m->y, m->b, m->p);
-	return m;
 }
 
 void
-msgdecref(Message *m)
+screenrot(Tablet *t)
 {
-	if(decref(m) == 0) {
-		free(m->msg);
-		free(m);
+	char *ln;
+	Biobuf *fd;
+
+	fd = Bopen("/dev/vgactl", OREAD);
+	if(!fd) {
+		t->srot = None;
+		return;
 	}
-}
 
-struct QItem {
-	Message *m;
-	QItem *next;
-};
-
-struct Queue {
-	Lock;
-	QItem *first, *last;
-};
-
-void
-qput(Queue* q, Message* m)
-{
-	QItem *i;
-	
-	lock(q);
-	i = malloc(sizeof(QItem));
-	i->m = m;
-	i->next = 0;
-	if(q->last == nil) {
-		q->last = q->first = i;
-	} else {
-		q->last->next = i;
-		q->last = i;
+	while(ln = Brdstr(fd, '\n', 1)) {
+		if(strncmp("tilt ", ln, 5) == 0)
+			switch(ln[5]) {
+			case 'l':
+				t->srot = Left;
+				break;
+			case 'r':
+				t->srot = Right;
+				break;
+			case 'i':
+				t->srot = Inverted;
+				break;
+			default:
+				t->srot = None;
+			}
+		free(ln);
 	}
-	unlock(q);
-}
-
-Message*
-qget(Queue* q)
-{
-	QItem *i;
-	Message *m;
-	
-	if(q->first == nil) return nil;
-	lock(q);
-	i = q->first;
-	if(q->first == q->last) {
-		q->first = q->last = nil;
-	} else {
-		q->first = i->next;
-	}
-	m = i->m;
-	free(i);
-	unlock(q);
-	return m;
+	Bterm(fd);
 }
 
 void
-freequeue(Queue *q)
+updateproc(void *aux)
 {
-	Message *m;
-	
-	while(m = qget(q))
-		msgdecref(m);
-	free(q);
-}
+	Tablet t;
+	Channel *c;
 
-struct Reader {
-	Queue *e;
-	Reader *prev, *next;
-	Req* req;
-};
+	c = aux;
+	recv(c, &t);
+	t.b = 0;
+	t.x = 0;
+	t.y = 0;
+	t.p = 0;
 
-Lock readers;
-Reader *rfirst, *rlast;
+	setuptablet(&t);
+	query(&t);
 
-void
-reply(Req *req, Message *m)
-{
-	req->ofcall.count = strlen(m->msg);
-	if(req->ofcall.count > req->ifcall.count)
-		req->ofcall.count = req->ifcall.count;
-	memmove(req->ofcall.data, m->msg, req->ofcall.count);
-	respond(req, nil);
-}
-
-void
-sendout(Message *m)
-{
-	Reader *r;
-	
-	lock(&readers);
-	for(r = rfirst; r; r = r->next) {
-		if(r->req) {
-			reply(r->req, m);
-			r->req = nil;
-		} else {
-			incref(m);
-			qput(r->e, m);
-		}
-	}
-	unlock(&readers);
-}
-
-void
-tabletopen(Req *req)
-{
-	Reader *r;
-	
-	lock(&readers);
-	r = calloc(sizeof(Reader), 1);
-	r->e = calloc(sizeof(Queue), 1);
-	if(rlast) rlast->next = r;
-	r->prev = rlast;
-	rlast = r;
-	if(rfirst == nil) rfirst = r;
-	unlock(&readers);
-	req->fid->aux = r;
-	respond(req, nil);
-}
-
-void
-tabletdestroyfid(Fid *fid)
-{
-	Reader *r;
-
-	r = fid->aux;
-	if(r == nil) return;
-	lock(&readers);
-	if(r->prev) r->prev->next = r->next;
-	if(r->next) r->next->prev = r->prev;
-	if(r == rfirst) rfirst = r->next;
-	if(r == rlast) rlast = r->prev;
-	freequeue(r->e);
-	free(r);
-	unlock(&readers);
-}
-
-void
-tabletdestroyreq(Req *req)
-{
-	Reader *r;
-	
-	if(req->fid == nil) return;
-	r = req->fid->aux;
-	if(r == nil) return;
-	if(req == r->req) {
-		r->req = nil;
+	for(;;) {
+		screensize(&t);
+		screenrot(&t);
+		readpacket(&t);
+		send(c, &t);
 	}
 }
 
 void
-tabletread(Req* req)
+scaleinput(Tablet *t)
 {
-	Reader *r;
-	Message *m;
-	
-	r = req->fid->aux;
-	if(m = qget(r->e)) {
-		reply(req, m);
-		msgdecref(m);
-	} else {
-		if(r->req) {
-			respond(req, "no concurrent reads, please");
-		} else {
-			r->req = req;
-		}
+	int temp;
+
+	t->p *= MAX;
+	t->p /= t->pmax;
+
+	switch(t->srot) {
+	case Left:
+		/* flip */
+		t->y = t->ymax - t->y;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->ymax;
+		t->y *= t->sy;
+		t->y /= t->xmax;
+		/* swap */
+		temp = t->x;
+		t->x = t->y;
+		t->y = temp;
+		break;
+	case Right:
+		/* flip */
+		t->x = t->xmax - t->x;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->ymax;
+		t->y *= t->sy;
+		t->y /= t->xmax;
+		/* swap */
+		temp = t->x;
+		t->x = t->y;
+		t->y = temp;
+		break;
+	case Inverted:
+		/* flip */
+		t->y = t->ymax - t->y;
+		t->x = t->xmax - t->x;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->xmax;
+		t->y *= t->sy;
+		t->y /= t->ymax;
+		break;
+	default:
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->xmax;
+		t->y *= t->sy;
+		t->y /= t->ymax;
 	}
 }
 
-Srv tabletsrv = {
-	.open = tabletopen,	
-	.read = tabletread,
-	.destroyfid = tabletdestroyfid,
-	.destroyreq = tabletdestroyreq,
+void
+replyproc(void *aux)
+{
+	Req *req;
+	Channel *c;
+	Tablet t;
+	char *reply;
+
+	req = aux;
+	c = req->srv->aux;
+	recv(c, &t);
+	scaleinput(&t);
+
+	reply = smprint("m %d %d %d %d\n",
+		t.x, t.y, t.b, t.p);
+
+	req->ofcall.count = strlen(reply);
+	if(req->ofcall.count > req->ifcall.count)
+		req->ofcall.count = req->ifcall.count;
+	memmove(req->ofcall.data, reply, req->ofcall.count);
+	respond(req, nil);
+
+	free(reply);
+}
+
+void
+fsread(Req *req)
+{
+	proccreate(replyproc, req, STACK);
+}
+
+void
+fsflush(Req *req)
+{
+	proccreate(replyproc, req->oldreq, STACK);
+}
+
+Srv fs = {
+	.read = fsread,
+	.flush = fsflush,
 };
 
-File *tfile;
+void
+srvproc(void *aux)
+{
+	Channel *c;
+	
+	c = aux;
+	fs.aux = c;
+	fs.tree = alloctree(getuser(), getuser(), 0555, nil);
+	createfile(fs.tree->root, "tablet", getuser(), 0400, nil);
+	threadpostmountsrv(&fs, nil, "/dev", MAFTER);
+}
 
 void
-main()
+usage(void)
 {
-	Tablet *t;
-	Message *m;
-	int fd[2];
-	
-	pipe(fd);
-	tabletsrv.infd = tabletsrv.outfd = fd[0];
-	tabletsrv.srvfd = fd[1];
-	tabletsrv.tree = alloctree(getuser(), getuser(), 0555, 0);
-	tfile = createfile(tabletsrv.tree->root, "tablet", getuser(), 0400, 0);
-	if(rfork(RFPROC | RFMEM | RFNOWAIT | RFNOTEG) > 0) exits(nil);
-	if(rfork(RFPROC | RFMEM) == 0) {
-		srv(&tabletsrv);
-		exits(nil);
-	}
-	mount(fd[1], -1, "/dev", MAFTER, "");
-	
-	t = newtablet("/dev/eia2");
-	if(!t) sysfatal("%r");
-	if(screensize(t) < 0) sysfatal("%r");
-	if(query(t) < 0) sysfatal("%r");
-	while(1) {
-		m = readpacket(t);
-		if(!m) sysfatal("%r");
-		sendout(m);
-		msgdecref(m);
-	}
-}
\ No newline at end of file
+	fprint(2, "usage: %s [-d dev]\n", argv0);
+	exits("usage");
+}
+
+void
+threadmain(int argc, char *argv[])
+{
+	Channel *c;
+	Tablet t;
+
+	ARGBEGIN {
+	case 'd':
+		strncmp(t.dev, EARGF(usage()), sizeof(t.dev));
+		break;
+	default:
+		usage();
+	} ARGEND
+
+	c = chancreate(sizeof(Tablet), 10);
+	strncpy(t.dev, "/dev/eia2", sizeof(t.dev));
+
+	proccreate(updateproc, c, STACK);
+	send(c, &t);
+	proccreate(srvproc, c, STACK);
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-11 15:39 ` [9front] " james palmer
@ 2021-04-12  7:37   ` hiro
  2021-04-12 10:00     ` james palmer
  2021-04-12  8:12   ` cinap_lenrek
  1 sibling, 1 reply; 13+ messages in thread
From: hiro @ 2021-04-12  7:37 UTC (permalink / raw)
  To: 9front

> i fixed walks of /dev blocking by launching a new proc for each 9p request.
> (i should probably convert this to use a Reqqueue, that will come soon)

why the reqqueue? bec. of mouse movement events getting reordered?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-11 15:39 ` [9front] " james palmer
  2021-04-12  7:37   ` hiro
@ 2021-04-12  8:12   ` cinap_lenrek
  2021-04-12  9:57     ` james palmer
  2021-04-19 15:49     ` james palmer
  1 sibling, 2 replies; 13+ messages in thread
From: cinap_lenrek @ 2021-04-12  8:12 UTC (permalink / raw)
  To: 9front

the way todo this is using srvrelease()/srvacquire():

          Srvrelease temporarily releases the calling process from the
          server loop and if neccesary spawns a new process to handle
          9p requests. When released, the process can do blocking work
          that would otherwise halt processing of 9p requests.
          Srvacquire rejoins the calling process with the server loop
          after a srvrelease.

also, what i dont like is polling /dev/vgactl. the tiling
feature is currently pc specific as other ports do not have
a vga device.

it is not obvious to me why tiling the screen would need
also rotating the tablet axis. for example, tiling might
just be used to compensate on how the display panel got
mounted in a laptop. the panel should just be scaled
to the *logical* screen dimensions. the kernels devmouse
driver already allows this by having a scaled versions
of the mousein command ("a" command, see scmousetrack()
in /sys/src/9/port/devmouse.c).

in any case, if we need axis change for tablets, we should
have this as a separate setting... independent of devvga.

--
cinap

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12  8:12   ` cinap_lenrek
@ 2021-04-12  9:57     ` james palmer
  2021-04-19 15:49     ` james palmer
  1 sibling, 0 replies; 13+ messages in thread
From: james palmer @ 2021-04-12  9:57 UTC (permalink / raw)
  To: 9front mailing list

Quoth cinap_lenrek@felloff.net:
> also, what i dont like is polling /dev/vgactl. the tiling
> feature is currently pc specific as other ports do not have a vga device.
> 
> it is not obvious to me why tiling the screen would need also rotating the tablet axis.

this driver is specific to wacom digitiser based tablet pcs, such as my thinkpad x41t. it is desireable for the mouse cursor to line up with where you point the pen on the screen.

thanks for the advice on using srvrelease() and srvaquire().

- james

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12  7:37   ` hiro
@ 2021-04-12 10:00     ` james palmer
  2021-04-12 17:03       ` hiro
  0 siblings, 1 reply; 13+ messages in thread
From: james palmer @ 2021-04-12 10:00 UTC (permalink / raw)
  To: 9front mailing list

Quoth 23hiro@gmail.com:
> why the reqqueue? bec. of mouse movement events getting reordered?

i figured launching a new process every time the pen moves is a bad idea.
it will increment the pid very quickly.

- james

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12 10:00     ` james palmer
@ 2021-04-12 17:03       ` hiro
  2021-04-15 21:09         ` cinap_lenrek
  2021-04-24 19:57         ` Michael Enders
  0 siblings, 2 replies; 13+ messages in thread
From: hiro @ 2021-04-12 17:03 UTC (permalink / raw)
  To: 9front

yeah, that's an interesting question.

maybe we need to implement pid reusal...
maybe nowadays it would be fine to be wasteful about this.

and where does my hunch come from? werc also wastes processes, and yet
everybody seems happy to use it.

On 4/12/21, james palmer <james@biobuf.link> wrote:
> Quoth 23hiro@gmail.com:
>> why the reqqueue? bec. of mouse movement events getting reordered?
>
> i figured launching a new process every time the pen moves is a bad idea.
> it will increment the pid very quickly.
>
> - james
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12 17:03       ` hiro
@ 2021-04-15 21:09         ` cinap_lenrek
  2021-04-16  9:25           ` hiro
  2021-04-24 19:57         ` Michael Enders
  1 sibling, 1 reply; 13+ messages in thread
From: cinap_lenrek @ 2021-04-15 21:09 UTC (permalink / raw)
  To: 9front

> maybe we need to implement pid reusal...

already done. wrapping pid's should not be a problem.

the main concern is that creating and destroying processes
has a cost compared to just wake up a existing process.

--
cinap

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-15 21:09         ` cinap_lenrek
@ 2021-04-16  9:25           ` hiro
  2021-04-17  3:32             ` cinap_lenrek
  0 siblings, 1 reply; 13+ messages in thread
From: hiro @ 2021-04-16  9:25 UTC (permalink / raw)
  To: 9front

yeah, i was wondering if that was the motivation here, esp. in regards
to minimizing user perceived latency.

and if yes i'd be curious if it's actually significant enough to matter.

i can imagine high throughput scenarios, just not mouse input, where
this seems safe to assume even without measuring :D

On 4/15/21, cinap_lenrek@felloff.net <cinap_lenrek@felloff.net> wrote:
>> maybe we need to implement pid reusal...
>
> already done. wrapping pid's should not be a problem.
>
> the main concern is that creating and destroying processes
> has a cost compared to just wake up a existing process.
>
> --
> cinap
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-16  9:25           ` hiro
@ 2021-04-17  3:32             ` cinap_lenrek
  0 siblings, 0 replies; 13+ messages in thread
From: cinap_lenrek @ 2021-04-17  3:32 UTC (permalink / raw)
  To: 9front

> yeah, i was wondering if that was the motivation here, esp.

my motivation to respond to this was the concern about wrapping pids.
which is some area we actively spend time writing code to solve the
issue.

> in regards to minimizing user perceived latency.

also think about reordering. when you spawn processes for each message,
the oder of execution of them is not controllable, as the scheduler
(on each cpu) is free to pick any to run next.

> and if yes i'd be curious if it's actually significant enough to matter.

it is just stupid. dont do stupid things. it doesnt make anything
easier to spawn a process for every message. and you'r going to run
into resource exhaustion problems as soon as theres anything blocking
in the execution of these procs for any reason.

any kind of unbounded forking can lead to problems. just like unbounded
queues. everything might work fine in normal case... until the system
experiences some kind of blockage and then collapses.

> i can imagine high throughput scenarios, just not mouse input, where
> this seems safe to assume even without measuring :D

this has absolutely NOTHING todo with bandwidth. at all.

the (process) switching and messaging overhead is always the same
and something you cant get rid of. a fork takes the same amount of
time no matter if you call your program "high bandwidth" or "low bandwidth".

> On 4/15/21, cinap_lenrek@felloff.net <cinap_lenrek@felloff.net> wrote:
>>> maybe we need to implement pid reusal...
>>
>> already done. wrapping pid's should not be a problem.
>>
>> the main concern is that creating and destroying processes
>> has a cost compared to just wake up a existing process.

--
cinap

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12  8:12   ` cinap_lenrek
  2021-04-12  9:57     ` james palmer
@ 2021-04-19 15:49     ` james palmer
  1 sibling, 0 replies; 13+ messages in thread
From: james palmer @ 2021-04-19 15:49 UTC (permalink / raw)
  To: 9front

Quoth cinap_lenrek@felloff.net:
> the way todo this is using srvrelease()/srvacquire():
> 
>           Srvrelease temporarily releases the calling process from the
>           server loop and if neccesary spawns a new process to handle
>           9p requests. When released, the process can do blocking work
>           that would otherwise halt processing of 9p requests.
>           Srvacquire rejoins the calling process with the server loop
>           after a srvrelease.

i have updated this to use srvrelease() and srvacquire().
polling of /dev/vgactl has been kept because this driver is specific to tablet pcs;
you would always want the mouse cursor to line up with where the pen is pointed on the screen.

let me know if you have any other issues with it.

thanks,
- james


# HG changeset patch
# User foura <james@biobuf.link>
# Date 1618846948 -3600
# Node ID 7e0db6f829a50e8c1af98a3be6600aef8436a628
# Parent  b31369df5d7d30ceb3bc104d9d4a4a3dba989b12
aux/wacom: rewrite using multiple procs. allow for screen tilt.

fixes "no concurrent reads please" when moving the pen too quickly.
rotates the tablet input with the screen tilt so that the pointer is always under the pen position.

diff -r b31369df5d7d -r 7e0db6f829a5 sys/src/cmd/aux/wacom.c
--- a/sys/src/cmd/aux/wacom.c	Sun Apr 18 16:20:04 2021 +0200
+++ b/sys/src/cmd/aux/wacom.c	Mon Apr 19 16:42:28 2021 +0100
@@ -1,352 +1,309 @@
 #include <u.h>
 #include <libc.h>
+#include <bio.h>
+#include <thread.h>
 #include <fcall.h>
-#include <thread.h>
 #include <9p.h>
 
-typedef struct Tablet Tablet;
-typedef struct Message Message;
-typedef struct QItem QItem;
-typedef struct Queue Queue;
-typedef struct Reader Reader;
+enum {
+	None,
+	Left,
+	Right,
+	Inverted,
 
-
-enum { MAX = 1000 };
-
-struct Tablet {
-	int ser;
-	int xmax, ymax, pmax, version;
-	int sx, sy;
+	MAX = 1000,
+	STACK = 2048,
 };
 
-struct Message {
-	Ref;
-	int b, x, y, p;
-	char *msg;
+typedef struct Tablet Tablet;
+struct Tablet {
+
+	int fd;
+	char dev[64];
+	int version;
+
+	int x, y, b, p;
+	int xmax, ymax, pmax;
+
+	int sx, sy, srot;
 };
 
-Tablet*
-newtablet(char* dev)
+void
+setuptablet(Tablet *t)
 {
-	int serctl;
-	char* ctl;
-	Tablet* t;
+	int ctlfd;
+	char *ctl;
+	
+	ctl = smprint("%sctl", t->dev);
+	t->fd = open(t->dev, ORDWR);
+	if(t->fd < 0)
+		sysfatal("%r");
+	
+	ctlfd = open(ctl, OWRITE);
+	if(ctlfd < 0)
+		sysfatal("%r");
+	free(ctl);
 
-	ctl = smprint("%sctl", dev);
-	t = calloc(sizeof(Tablet), 1);
-	t->ser = open(dev, ORDWR);
-	if(t->ser < 0) {
-		free(t);
-		return 0;
-	}
-	serctl = open(ctl, OWRITE);
-	free(ctl);
-	if(serctl < 0) {
-		free(t);
-		close(t->ser);
-		return 0;
-	}
-	if(fprint(serctl, "b19200\n") < 0) {
-		free(t);
-		close(t->ser);
-		close(serctl);
-		return 0;
-	}
-	close(serctl);
-	return t;
+	if(fprint(ctlfd, "b19200\n") < 0)
+		sysfatal("set baud rate: %r");
 }
 
 int
-query(Tablet* t)
+query(Tablet *t)
 {
 	uchar buf[11];
 
-	if(write(t->ser, "&0*", 3) < 3) return -1;
+	if(write(t->fd, "&0*", 3) < 3) return 0;
 	do {
-		if(read(t->ser, buf, 1) < 1) return -1;
+		if(read(t->fd, buf, 1) < 1) return 0;
 	} while(buf[0] != 0xC0);
-	if(readn(t->ser, buf+1, 10) < 10) return -1;
+
+	if(readn(t->fd, buf+1, 10) < 10) return 0;
+
 	t->xmax = (buf[1] << 9) | (buf[2] << 2) | ((buf[6] >> 5) & 3);
 	t->ymax = (buf[3] << 9) | (buf[4] << 2) | ((buf[6] >> 3) & 3);
 	t->pmax = buf[5] | (buf[6] & 7);
 	t->version = (buf[9] << 7) | buf[10];
-	if(write(t->ser, "1", 1) < 1) return -1;
-	return 0;
+
+	if(write(t->fd, "1", 1) < 1) return 0;
+	return 1;
 }
 
 int
-screensize(Tablet* t)
+findheader(Tablet *t)
+{
+	uchar c;
+
+	do {
+		if(read(t->fd, &c, 1) < 1) return -1;
+	} while((c & 0x80) == 0);
+
+	return c;
+}
+
+void
+readpacket(Tablet *t)
+{
+	uchar buf[9];
+	int head;
+
+	head = findheader(t);
+	if(head < 0) sysfatal("%r");
+	if(readn(t->fd, buf, 9) < 9) sysfatal("%r");
+
+	t->b = head & 7;
+	t->x = (buf[0] << 9) | (buf[1] << 2) | ((buf[5] >> 5) & 3);
+	t->y = (buf[2] << 9) | (buf[3] << 2) | ((buf[5] >> 3) & 3);
+	t->p = ((buf[5] & 7) << 7) | buf[4];
+}
+
+void
+screensize(Tablet *t)
 {
 	int fd;
 	char buf[189], buf2[12], *p;
-	
+
 	fd = open("/dev/draw/new", OREAD);
-	if(fd < 0) return -1;
+	if(fd < 0) sysfatal("%r");
 	read(fd, buf, 189);
+
 	memcpy(buf2, buf + 72, 11);
 	buf2[11] = 0;
 	for(p = buf2; *p == ' '; p++);
 	t->sx = atoi(p);
+
 	memcpy(buf2, buf + 84, 11);
 	for(p = buf2; *p == ' '; p++);
 	t->sy = atoi(p);
+
 	if(t->sx == 0 || t->sy == 0) {
-		close(fd);
-		werrstr("invalid resolution read from /dev/draw/new");
-		return -1;
+		sysfatal("invalid resolution read from /dev/draw/new");
 	}
-	
+
 	close(fd);
-	return 0;
-}
-
-int
-findheader(Tablet* t)
-{
-	uchar c;
-	
-	do {
-		if(read(t->ser, &c, 1) < 1) return -1;
-	} while((c & 0x80) == 0);
-	return c;
-}
-
-Message*
-readpacket(Tablet* t)
-{
-	uchar buf[9];
-	int head;
-	Message *m;
-
-	head = findheader(t);
-	if(head < 0) return 0;
-	if(readn(t->ser, buf, 9) < 9) return 0;
-	
-	m = calloc(sizeof(Message), 1);
-	incref(m);
-	
-	m->b = head & 7;
-	m->x = (buf[0] << 9) | (buf[1] << 2) | ((buf[5] >> 5) & 3);
-	m->y = (buf[2] << 9) | (buf[3] << 2) | ((buf[5] >> 3) & 3);
-	m->p = ((buf[5] & 7) << 7) | buf[4];
-	
-	m->p *= MAX;
-	m->p /= t->pmax;
-	m->x *= t->sx;
-	m->x /= t->xmax;
-	m->y *= t->sy;
-	m->y /= t->ymax;
-	
-	m->msg = smprint("m %d %d %d %d\n", m->x, m->y, m->b, m->p);
-	return m;
 }
 
 void
-msgdecref(Message *m)
+screenrot(Tablet *t)
 {
-	if(decref(m) == 0) {
-		free(m->msg);
-		free(m);
+	char *ln;
+	Biobuf *fd;
+
+	fd = Bopen("/dev/vgactl", OREAD);
+	if(!fd) {
+		t->srot = None;
+		return;
 	}
-}
 
-struct QItem {
-	Message *m;
-	QItem *next;
-};
-
-struct Queue {
-	Lock;
-	QItem *first, *last;
-};
-
-void
-qput(Queue* q, Message* m)
-{
-	QItem *i;
-	
-	lock(q);
-	i = malloc(sizeof(QItem));
-	i->m = m;
-	i->next = 0;
-	if(q->last == nil) {
-		q->last = q->first = i;
-	} else {
-		q->last->next = i;
-		q->last = i;
+	while(ln = Brdstr(fd, '\n', 1)) {
+		if(strncmp("tilt ", ln, 5) == 0)
+			switch(ln[5]) {
+			case 'l':
+				t->srot = Left;
+				break;
+			case 'r':
+				t->srot = Right;
+				break;
+			case 'i':
+				t->srot = Inverted;
+				break;
+			default:
+				t->srot = None;
+			}
+		free(ln);
 	}
-	unlock(q);
-}
-
-Message*
-qget(Queue* q)
-{
-	QItem *i;
-	Message *m;
-	
-	if(q->first == nil) return nil;
-	lock(q);
-	i = q->first;
-	if(q->first == q->last) {
-		q->first = q->last = nil;
-	} else {
-		q->first = i->next;
-	}
-	m = i->m;
-	free(i);
-	unlock(q);
-	return m;
+	Bterm(fd);
 }
 
 void
-freequeue(Queue *q)
+updateproc(void *aux)
 {
-	Message *m;
-	
-	while(m = qget(q))
-		msgdecref(m);
-	free(q);
-}
+	Tablet t;
+	Channel *c;
 
-struct Reader {
-	Queue *e;
-	Reader *prev, *next;
-	Req* req;
-};
+	c = aux;
+	recv(c, &t);
+	t.b = 0;
+	t.x = 0;
+	t.y = 0;
+	t.p = 0;
 
-Lock readers;
-Reader *rfirst, *rlast;
+	setuptablet(&t);
+	query(&t);
 
-void
-reply(Req *req, Message *m)
-{
-	req->ofcall.count = strlen(m->msg);
-	if(req->ofcall.count > req->ifcall.count)
-		req->ofcall.count = req->ifcall.count;
-	memmove(req->ofcall.data, m->msg, req->ofcall.count);
-	respond(req, nil);
-}
-
-void
-sendout(Message *m)
-{
-	Reader *r;
-	
-	lock(&readers);
-	for(r = rfirst; r; r = r->next) {
-		if(r->req) {
-			reply(r->req, m);
-			r->req = nil;
-		} else {
-			incref(m);
-			qput(r->e, m);
-		}
-	}
-	unlock(&readers);
-}
-
-void
-tabletopen(Req *req)
-{
-	Reader *r;
-	
-	lock(&readers);
-	r = calloc(sizeof(Reader), 1);
-	r->e = calloc(sizeof(Queue), 1);
-	if(rlast) rlast->next = r;
-	r->prev = rlast;
-	rlast = r;
-	if(rfirst == nil) rfirst = r;
-	unlock(&readers);
-	req->fid->aux = r;
-	respond(req, nil);
-}
-
-void
-tabletdestroyfid(Fid *fid)
-{
-	Reader *r;
-
-	r = fid->aux;
-	if(r == nil) return;
-	lock(&readers);
-	if(r->prev) r->prev->next = r->next;
-	if(r->next) r->next->prev = r->prev;
-	if(r == rfirst) rfirst = r->next;
-	if(r == rlast) rlast = r->prev;
-	freequeue(r->e);
-	free(r);
-	unlock(&readers);
-}
-
-void
-tabletdestroyreq(Req *req)
-{
-	Reader *r;
-	
-	if(req->fid == nil) return;
-	r = req->fid->aux;
-	if(r == nil) return;
-	if(req == r->req) {
-		r->req = nil;
+	for(;;) {
+		screensize(&t);
+		screenrot(&t);
+		readpacket(&t);
+		send(c, &t);
 	}
 }
 
 void
-tabletread(Req* req)
+scaleinput(Tablet *t)
 {
-	Reader *r;
-	Message *m;
-	
-	r = req->fid->aux;
-	if(m = qget(r->e)) {
-		reply(req, m);
-		msgdecref(m);
-	} else {
-		if(r->req) {
-			respond(req, "no concurrent reads, please");
-		} else {
-			r->req = req;
-		}
+	int temp;
+
+	t->p *= MAX;
+	t->p /= t->pmax;
+
+	switch(t->srot) {
+	case Left:
+		/* flip */
+		t->y = t->ymax - t->y;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->ymax;
+		t->y *= t->sy;
+		t->y /= t->xmax;
+		/* swap */
+		temp = t->x;
+		t->x = t->y;
+		t->y = temp;
+		break;
+	case Right:
+		/* flip */
+		t->x = t->xmax - t->x;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->ymax;
+		t->y *= t->sy;
+		t->y /= t->xmax;
+		/* swap */
+		temp = t->x;
+		t->x = t->y;
+		t->y = temp;
+		break;
+	case Inverted:
+		/* flip */
+		t->y = t->ymax - t->y;
+		t->x = t->xmax - t->x;
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->xmax;
+		t->y *= t->sy;
+		t->y /= t->ymax;
+		break;
+	default:
+		/* scale */
+		t->x *= t->sx;
+		t->x /= t->xmax;
+		t->y *= t->sy;
+		t->y /= t->ymax;
 	}
 }
 
-Srv tabletsrv = {
-	.open = tabletopen,	
-	.read = tabletread,
-	.destroyfid = tabletdestroyfid,
-	.destroyreq = tabletdestroyreq,
+void
+fsread(Req *req)
+{
+	Channel *c;
+	Tablet t;
+	char *reply;
+
+	c = req->srv->aux;
+
+    srvrelease(req->srv);
+	recv(c, &t);
+	scaleinput(&t);
+    srvacquire(req->srv);
+
+	reply = smprint("m %d %d %d %d\n",
+		t.x, t.y, t.b, t.p);
+
+	req->ofcall.count = strlen(reply);
+	if(req->ofcall.count > req->ifcall.count)
+		req->ofcall.count = req->ifcall.count;
+	memmove(req->ofcall.data, reply, req->ofcall.count);
+	respond(req, nil);
+
+	free(reply);
+}
+
+Srv fs = {
+	.read = fsread,
 };
 
-File *tfile;
+void
+srvproc(void *aux)
+{
+	Channel *c;
+	
+	c = aux;
+	fs.aux = c;
+	fs.tree = alloctree(getuser(), getuser(), 0555, nil);
+	createfile(fs.tree->root, "tablet", getuser(), 0400, nil);
+
+	threadpostmountsrv(&fs, nil, "/dev", MAFTER);
+}
 
 void
-main()
+usage(void)
 {
-	Tablet *t;
-	Message *m;
-	int fd[2];
-	
-	pipe(fd);
-	tabletsrv.infd = tabletsrv.outfd = fd[0];
-	tabletsrv.srvfd = fd[1];
-	tabletsrv.tree = alloctree(getuser(), getuser(), 0555, 0);
-	tfile = createfile(tabletsrv.tree->root, "tablet", getuser(), 0400, 0);
-	if(rfork(RFPROC | RFMEM | RFNOWAIT | RFNOTEG) > 0) exits(nil);
-	if(rfork(RFPROC | RFMEM) == 0) {
-		srv(&tabletsrv);
-		exits(nil);
-	}
-	mount(fd[1], -1, "/dev", MAFTER, "");
-	
-	t = newtablet("/dev/eia2");
-	if(!t) sysfatal("%r");
-	if(screensize(t) < 0) sysfatal("%r");
-	if(query(t) < 0) sysfatal("%r");
-	while(1) {
-		m = readpacket(t);
-		if(!m) sysfatal("%r");
-		sendout(m);
-		msgdecref(m);
-	}
-}
\ No newline at end of file
+	fprint(2, "usage: %s [-D] [-d dev]\n", argv0);
+	exits("usage");
+}
+
+void
+threadmain(int argc, char *argv[])
+{
+	Channel *c;
+	Tablet t;
+
+	ARGBEGIN {
+	case 'd':
+		strncmp(t.dev, EARGF(usage()), sizeof(t.dev));
+		break;
+	case 'D':
+		chatty9p++;
+		break;
+	default:
+		usage();
+	} ARGEND
+
+	c = chancreate(sizeof(Tablet), 10);
+	strncpy(t.dev, "/dev/eia2", sizeof(t.dev));
+
+	proccreate(updateproc, c, STACK);
+	send(c, &t);
+	proccreate(srvproc, c, STACK);
+}

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt
  2021-04-12 17:03       ` hiro
  2021-04-15 21:09         ` cinap_lenrek
@ 2021-04-24 19:57         ` Michael Enders
  1 sibling, 0 replies; 13+ messages in thread
From: Michael Enders @ 2021-04-24 19:57 UTC (permalink / raw)
  To: 9front


>
> maybe we need to implement pid reusal...
> maybe nowadays it would be fine to be wasteful about this.

What's the issue with wrapping pids?

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-04-25  2:49 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-10 15:39 [9front] [patch] aux/wacom: prevent "no concurrent reads please" error when moving the pen too fast and support screen tilt james palmer
     [not found] ` <b0646909-d59a-4423-8467-32c4c96ef5d3@www.fastmail.com>
2021-04-10 16:49   ` [9front] " james palmer
2021-04-11 15:39 ` [9front] " james palmer
2021-04-12  7:37   ` hiro
2021-04-12 10:00     ` james palmer
2021-04-12 17:03       ` hiro
2021-04-15 21:09         ` cinap_lenrek
2021-04-16  9:25           ` hiro
2021-04-17  3:32             ` cinap_lenrek
2021-04-24 19:57         ` Michael Enders
2021-04-12  8:12   ` cinap_lenrek
2021-04-12  9:57     ` james palmer
2021-04-19 15:49     ` james palmer

9front - general discussion about 9front

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/9front

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 9front 9front/ http://inbox.vuxu.org/9front \
		9front@9front.org
	public-inbox-index 9front

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.9front


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git