Messages in this thread | | | Subject | Re: [PATCH v0 1/5] Change/add data structures for SRG mouse reporting | From | Jiri Slaby <> | Date | Fri, 26 Jun 2020 12:38:03 +0200 |
| |
On 26. 06. 20, 9:22, Tammo Block wrote: > This extends the data structures and defines needed for additional > mouse reporting features. If URXVT and UTF8 reports should be supported > in the future, vc_proto_mouse would need another bit.
Just a global remark: I have just rewritten a large part of the vt code. You might need to rebase on the top of tty-next...
> Signed-off-by: Tammo Block <tammo.block@gmail.com> > --- > include/linux/console_struct.h | 3 ++- > include/uapi/linux/tiocl.h | 24 ++++++++++++++++-------- > 2 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/include/linux/console_struct.h b/include/linux/console_struct.h > index 24d4c16e3ae0..cfb581eb8b82 100644 > --- a/include/linux/console_struct.h > +++ b/include/linux/console_struct.h > @@ -122,7 +122,8 @@ struct vc_data { > unsigned int vc_priv : 3; > unsigned int vc_need_wrap : 1; > unsigned int vc_can_do_color : 1; > - unsigned int vc_report_mouse : 2; > + unsigned int vc_report_mouse : 3; > + unsigned int vc_proto_mouse : 1;
bitfields... The generated code is so bad (it does masks, shifts and ANDs in assembly), so please avoid them and define the new members the standard way. I.e. unsigned char if 8 bits are enough, bool for one bit and so on. The price is larger structure, but we can afford 5 more bits per virt console, I think. Provided there is a very limited number them -- this is not "struct page" after all :).
> unsigned char vc_utf : 1; /* Unicode UTF-8 encoding */ > unsigned char vc_utf_count; > int vc_utf_char; > diff --git a/include/uapi/linux/tiocl.h b/include/uapi/linux/tiocl.h > index b32acc229024..df27608648f7 100644 > --- a/include/uapi/linux/tiocl.h > +++ b/include/uapi/linux/tiocl.h > @@ -3,13 +3,17 @@ > #define _LINUX_TIOCL_H > > #define TIOCL_SETSEL 2 /* set a selection */ > -#define TIOCL_SELCHAR 0 /* select characters */ > -#define TIOCL_SELWORD 1 /* select whole words */ > -#define TIOCL_SELLINE 2 /* select whole lines */ > -#define TIOCL_SELPOINTER 3 /* show the pointer */ > -#define TIOCL_SELCLEAR 4 /* clear visibility of selection */ > -#define TIOCL_SELMOUSEREPORT 16 /* report beginning of selection */ > -#define TIOCL_SELBUTTONMASK 15 /* button mask for report */ > +#define TIOCL_SELCHAR 0 /* select characters */ > +#define TIOCL_SELWORD 1 /* select whole words */ > +#define TIOCL_SELLINE 2 /* select whole lines */ > +#define TIOCL_SELPOINTER 3 /* show the pointer */ > +#define TIOCL_SELCLEAR 4 /* clear visibility of selection */ > +#define TIOCL_SELMOUSEREPORT 16 /* send X10 mouse report */ > +#define TIOCL_SELBUTTONMASK 15 /* button mask for X10 report */ > +#define TIOCL_SELSRGREPORT 32 /* send SRG mouse report */ > +#define TIOCL_SELSRGRELEASE 128 /* SRG report is release event */ > +#define TIOCL_SELSRGMASK (255 << 8) /* mask for SRG report */
This right shift of all of them feels like too much right. Why do you do it? And if you do it, do it in a separate patch. It makes this one complicated to review.
> @@ -28,7 +32,11 @@ struct tiocl_selection { > > /* these two don't return a value: they write it back in the type */ > #define TIOCL_GETSHIFTSTATE 6 /* write shift state */ > -#define TIOCL_GETMOUSEREPORTING 7 /* write whether mouse event are reported */ > +#define TIOCL_GETMOUSEREPORTING 7 /* write which mouse event to be reported */ > +#define TIOCL_REPORTBTNPRESS 1 /* report button press only "9" */ > +#define TIOCL_REPORTRELEASE 2 /* report press and release "1000" */ > +#define TIOCL_REPORTDRAG 5 /* report drag events "1002" */ > +#define TIOCL_REPORTANYMOVE 6 /* report any movement "1003" */
Dtto.
> #define TIOCL_SETVESABLANK 10 /* set vesa blanking mode */ > #define TIOCL_SETKMSGREDIRECT 11 /* restrict kernel messages to a vt */ > #define TIOCL_GETFGCONSOLE 12 /* get foreground vt */
thanks, -- js
| |