[PATCH 1/1]: Fix poor performance during mouse mode selection.

Discussion:

Michael Graczyk

2014-12-09 01:25:29 UTC

On large displays, mouse selection performance was poor. Tmux was
redrawing the entire screen for every mouse coordinate update when it only
needed to update those lines where the selection might have changed.

This patch makes mouse selection buttery-smooth by only updating those
lines that need updating.

From aedcc3e976ad3df4037b479ac5b789160ab94a51 Mon Sep 17 00:00:00 2001

From: Michael Graczyk <***@mgraczyk.com>
Date: Mon, 8 Dec 2014 16:57:49 -0800
Subject: [PATCH] Fix poor performance during mouse mode selection.

---
window-copy.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/window-copy.c b/window-copy.c
index f597332..e7fbd32 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -868,12 +868,37 @@ window_copy_key_numeric_prefix(struct window_pane
*wp, int key)
return (0);
}

+static void
+window_copy_for_selection(
+ struct window_pane *wp,
+ u_int old_x,
+ u_int old_y
+) {
+ struct window_copy_mode_data *data = wp->modedata;
+ const u_int new_y = data->cy;
+ u_int start, end, lines;
+
+ (void)old_x;
+ if (old_y <= new_y) {
+ start = old_y;
+ end = new_y;
+ } else {
+ start = new_y;
+ end = old_y;
+ }
+
+ lines = end - start + 1;
+ window_copy_redraw_lines(wp, start, lines);
+}
+
void
window_copy_mouse(struct window_pane *wp, struct session *sess,
struct mouse_event *m)
{
struct window_copy_mode_data *data = wp->modedata;
struct screen *s = &data->screen;
+ u_int old_cx;
+ u_int old_cy;
u_int i;

if (m->x >= screen_size_x(s))
@@ -907,9 +932,11 @@ window_copy_mouse(struct window_pane *wp, struct
session *sess,
*/
if (s->mode & MODE_MOUSE_BUTTON) {
if (~m->event & MOUSE_EVENT_UP) {
+ old_cx = data->cx;
+ old_cy = data->cy;
window_copy_update_cursor(wp, m->x, m->y);
if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cx, old_cy);
return;
}
goto reset_mode;

--
2.2.0.rc0.207.ga3a616c

Nicholas Marriott

2014-12-09 09:37:01 UTC

Permalink

Your mailer has mangled this patch, can you send it as an attachment please?

Also please just remove the old_x argument if you don't need it (or mark
it "unused", but I'd just remove it). And there seems no need to use
temporaries for data->c[xy].

On large displays, mouse selection performance was poor.** Tmux was
redrawing the entire screen for every mouse coordinate update when it only
needed to update those lines where the selection might have changed.
This patch makes mouse selection buttery-smooth by only updating those
lines that need updating.
From aedcc3e976ad3df4037b479ac5b789160ab94a51 Mon Sep 17 00:00:00 2001
Date: Mon, 8 Dec 2014 16:57:49 -0800
Subject: [PATCH] Fix poor performance during mouse mode selection.
---
**window-copy.c | 29 ++++++++++++++++++++++++++++-
**1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/window-copy.c b/window-copy.c
index f597332..e7fbd32 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -868,12 +868,37 @@ window_copy_key_numeric_prefix(struct window_pane
*wp, int key)
** return (0);
**}
**
+static void
+window_copy_for_selection(
+ struct window_pane *wp,
+ u_int old_x,
+ u_int old_y
+) {
+ struct window_copy_mode_data *data = wp->modedata;
+ const u_int new_y = data->cy;
+ u_int start, end, lines;
+
+ (void)old_x;
+ if (old_y <= new_y) {
+ start = old_y;
+ end = new_y;
+ } else {
+ start = new_y;
+ end = old_y;
+ }
+
+ lines = end - start + 1;
+ window_copy_redraw_lines(wp, start, lines);
+}
+
**void
**window_copy_mouse(struct window_pane *wp, struct session *sess,
** ** **struct mouse_event *m)
**{
** struct window_copy_mode_data *data = wp->modedata;
** struct screen *s = &data->screen;
+ u_int old_cx;
+ u_int old_cy;
** u_int i;
**
** if (m->x >= screen_size_x(s))
@@ -907,9 +932,11 @@ window_copy_mouse(struct window_pane *wp, struct
session *sess,
** */
** if (s->mode & MODE_MOUSE_BUTTON) {
** if (~m->event & MOUSE_EVENT_UP) {
+ old_cx = data->cx;
+ old_cy = data->cy;
** window_copy_update_cursor(wp, m->x, m->y);
** if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cx,
old_cy);
** return;
** }
** goto reset_mode;
--**
2.2.0.rc0.207.ga3a616c
References
Visible links
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users

Michael Graczyk

2014-12-09 09:45:48 UTC

Permalink

I removed the unused argument and attached the patch. For future
reference, any easy way to get gmail to not mangle patches?

As for the data->cy temporary, the successive call
to window_copy_update_cursor clobbers the y coordinate in data. Unless the
y-coordinate is stored somewhere else (It doesn't appear to be), I have to
save a temp there.

Thanks,
Michael Graczyk

On Tue, Dec 9, 2014 at 1:37 AM, Nicholas Marriott <

Post by Nicholas Marriott
Your mailer has mangled this patch, can you send it as an attachment please?
Also please just remove the old_x argument if you don't need it (or mark
it "unused", but I'd just remove it). And there seems no need to use
temporaries for data->c[xy].

On large displays, mouse selection performance was poor.** Tmux was
redrawing the entire screen for every mouse coordinate update when it

only

needed to update those lines where the selection might have changed.
This patch makes mouse selection buttery-smooth by only updating those
lines that need updating.
From aedcc3e976ad3df4037b479ac5b789160ab94a51 Mon Sep 17 00:00:00 2001
Date: Mon, 8 Dec 2014 16:57:49 -0800
Subject: [PATCH] Fix poor performance during mouse mode selection.
---
**window-copy.c | 29 ++++++++++++++++++++++++++++-
**1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/window-copy.c b/window-copy.c
index f597332..e7fbd32 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -868,12 +868,37 @@ window_copy_key_numeric_prefix(struct

window_pane

*wp, int key)
** return (0);
**}
**
+static void
+window_copy_for_selection(
+ struct window_pane *wp,
+ u_int old_x,
+ u_int old_y
+) {
+ struct window_copy_mode_data *data = wp->modedata;
+ const u_int new_y = data->cy;
+ u_int start, end, lines;
+
+ (void)old_x;
+ if (old_y <= new_y) {
+ start = old_y;
+ end = new_y;
+ } else {
+ start = new_y;
+ end = old_y;
+ }
+
+ lines = end - start + 1;
+ window_copy_redraw_lines(wp, start, lines);
+}
+
**void
**window_copy_mouse(struct window_pane *wp, struct session *sess,
** ** **struct mouse_event *m)
**{
** struct window_copy_mode_data *data = wp->modedata;
** struct screen *s = &data->screen;
+ u_int old_cx;
+ u_int old_cy;
** u_int i;
**
** if (m->x >= screen_size_x(s))
@@ -907,9 +932,11 @@ window_copy_mouse(struct window_pane *wp, struct
session *sess,
** */
** if (s->mode & MODE_MOUSE_BUTTON) {
** if (~m->event & MOUSE_EVENT_UP) {
+ old_cx = data->cx;
+ old_cy = data->cy;
** window_copy_update_cursor(wp, m->x, m->y);
** if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cx,
old_cy);
** return;
** }
** goto reset_mode;
--**
2.2.0.rc0.207.ga3a616c
References
Visible links

------------------------------------------------------------------------------

Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE

http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk

_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users

Javier Tia

2014-12-10 14:50:14 UTC

Permalink

GMail does not have any way to turn off line wrapping in the web
interface, so it will mangle any emails that you send. You can however
use "git send-email" and send your patches through the GMail SMTP
server, or use any IMAP email client to connect to the google IMAP
server and forward the emails through that.

For hints on using git send-email to send your patches through the
GMail SMTP server, see the EXAMPLE section of git-send-email[1].

For hints on submission using the IMAP interface, see the EXAMPLE
section of git-imap-send[1].

HTH,

I removed the unused argument and attached the patch. For future reference,
any easy way to get gmail to not mangle patches?
As for the data->cy temporary, the successive call to
window_copy_update_cursor clobbers the y coordinate in data. Unless the
y-coordinate is stored somewhere else (It doesn't appear to be), I have to
save a temp there.
Thanks,
Michael Graczyk
On Tue, Dec 9, 2014 at 1:37 AM, Nicholas Marriott

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
tmux-users mailing list
https://lists.sourceforge.net/lists/listinfo/tmux-users

--
Javier

Thomas Adam

2014-12-10 15:12:20 UTC

Permalink

Post by Michael Graczyk
+static void

We don't mark functions as static in tmux; not even for the portable
version.

Post by Michael Graczyk
if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cy);

Rather than having a new function for this, why not put the logic into
window_copy_redraw_screen? The mode for the wp is already known.

-- Thomas Adam

Michael Graczyk

2014-12-11 00:33:51 UTC

Permalink

Whoops, last message had the wrong attachment. This is the correct one.

Thanks Javier for the information. Good to know.
Thomas,
I removed "static" from the declaration.
Bye the time window_copy_redraw_screen is called, the old data->cy has
already been clobbered so there is no way of determining which lines must
be withdrawn. I made window_copy_for_selection it's own function because
there are other places that it could save cycles (ie window_copy_goto_line, window_copy_scroll_to)
but those changes were outside the scope of this patch.
P.S. I noticed tmux does not use static functions but I couldn't find an
explanation in the repository. What's the reason for that?

Post by Thomas Adam

Post by Michael Graczyk
+static void

We don't mark functions as static in tmux; not even for the portable
version.

Post by Michael Graczyk
if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cy);

Rather than having a new function for this, why not put the logic into
window_copy_redraw_screen? The mode for the wp is already known.
-- Thomas Adam

Nicholas Marriott

2014-12-15 10:03:14 UTC

Permalink

Applied to OpenBSD now, will be in SF later, thanks.

Whoops, last message had the wrong attachment.** This is the correct one.
Thanks Javier for the information. Good to know.
Thomas,
I removed "static" from the declaration.
Bye the time window_copy_redraw_screen is called, the old data->cy has
already been clobbered so there is no way of determining which lines
must be withdrawn.** I made**window_copy_for_selection it's own function
because there are other places that it could save cycles
(ie**window_copy_goto_line,**window_copy_scroll_to) but those changes
were outside the scope of this patch.
P.S. I noticed tmux does not use static functions but I couldn't find an
explanation in the repository.** What's the reason for that?

Post by Michael Graczyk
+static void

We don't mark functions as static in tmux; not even for the portable
version.

Post by Michael Graczyk
** ** ** ** ** ** ** ** ** ** ** **if

(window_copy_update_selection(wp, 1))

Post by Michael Graczyk
-** ** ** ** ** ** ** ** ** ** ** ** ** **

**window_copy_redraw_screen(wp);

Post by Michael Graczyk
+** ** ** ** ** ** ** ** ** ** ** ** ** **

**window_copy_for_selection(wp, old_cy);
Rather than having a new function for this, why not put the logic into
window_copy_redraw_screen?** The mode for the wp is already known.
-- Thomas Adam
References
Visible links
From 2e20f9fce84f9fda16788a4cd26cd87cfce19cba Mon Sep 17 00:00:00 2001
Date: Mon, 8 Dec 2014 16:57:49 -0800
Subject: [PATCH] Fix poor performance during mouse mode selection.
---
window-copy.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/window-copy.c b/window-copy.c
index f597332..5f7b76a 100644
--- a/window-copy.c
+++ b/window-copy.c
@@ -869,11 +869,33 @@ window_copy_key_numeric_prefix(struct window_pane *wp, int key)
}
void
+window_copy_for_selection(
+ struct window_pane *wp,
+ u_int old_y
+) {
+ struct window_copy_mode_data *data = wp->modedata;
+ const u_int new_y = data->cy;
+ u_int start, end, lines;
+
+ if (old_y <= new_y) {
+ start = old_y;
+ end = new_y;
+ } else {
+ start = new_y;
+ end = old_y;
+ }
+
+ lines = end - start + 1;
+ window_copy_redraw_lines(wp, start, lines);
+}
+
+void
window_copy_mouse(struct window_pane *wp, struct session *sess,
struct mouse_event *m)
{
struct window_copy_mode_data *data = wp->modedata;
struct screen *s = &data->screen;
+ u_int old_cy;
u_int i;
if (m->x >= screen_size_x(s))
@@ -907,9 +929,10 @@ window_copy_mouse(struct window_pane *wp, struct session *sess,
*/
if (s->mode & MODE_MOUSE_BUTTON) {
if (~m->event & MOUSE_EVENT_UP) {
+ old_cy = data->cy;
window_copy_update_cursor(wp, m->x, m->y);
if (window_copy_update_selection(wp, 1))
- window_copy_redraw_screen(wp);
+ window_copy_for_selection(wp, old_cy);
return;
}
goto reset_mode;
--
2.2.0.rc0.207.ga3a616c