avoid heavy redrawing cursor #1873
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1873 +/- ##
==========================================
+ Coverage 75.04% 75.09% +0.05%
==========================================
Files 76 76
Lines 125234 125273 +39
==========================================
+ Hits 93984 94079 +95
+ Misses 31250 31194 -56
Continue to review full report at Codecov.
|
|
@brammool I found another solution. Which do you like? diff --git a/src/terminal.c b/src/terminal.c
index 5be51de9f..4ceb57cd9 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -636,7 +636,7 @@ handle_movecursor(
}
}
- if (is_current)
+ if (is_current && visible)
update_cursor();
return 1; |
|
I found an issue with this PR's (original) patch. For instance, when I do As for the diff that has just posted, I haven't seen the same issue so far. But, @mattn , I have one thing to make sure about the diff before giving my conclusion: Which HEAD should I apply it to? As no instructions are given, I applied it to the current master (8.0.0701) and tested it. But the line number 636 shown in the diff doesn't look like that the diff was made against the current HEAD, which makes me wonder. Or, does "another solution" mean that we should apply the diff on top of this PR's own patch? |
|
Did you try my first patch? or second one? The first patch is based on previous revision. Second one is based on master/HEAD. Additional: |
I did try the first patch and found the issue as I wrote.
Can I have its hash? I have no idea on what you are referring to "previous revision."
That's the point of my question. Actually, when I applied the posted patch to my clone repo points to master/HEAD, I got
It looks to me that the offset indicates that the diff was not made against master/HEAD. |
|
Sorry. I make you confusing. Okay, I'll rebase from master. |
mattn
force-pushed
the
heavy-cursor-redraw
branch
from
1ad38b3
to
2283a8c
Compare
|
OK, 'cause I have to be AFK for hours, let me leave my conclusion now:
Therefore, though I'm still unsure whether or not the second patch was made against master/HEAD and whether or not the executables resulting from the codebase of his repo which was used to make it are the same as those I have and were tested properly, I vote against the first patch and for the second patch, based on my own test result. |
|
@nuko8 Thanks for your review. As you mentioned about patch 8.0.0771, the cursor had appear in right location immediately. However, when lines are scrolled-up, cursor flicker many times. This patch is the avoid redrawing. To update cursor smooth, it should not call gui_update_cursor in many part. And it call gui_update_cursor at handle_movecursor while visible is true. |
|
Thank you for the clarification. Now I see the issue better and confirm that your second patch works just like you described. I highly appreciate your work on it as well as your recent intensive work on other part of the built-in terminal, helping Bram. Keep up the good work! |
|
Yasuhiro Matsumoto wrote:
@brammool I found another solution. Which do you like?
```diff
diff --git a/src/terminal.c b/src/terminal.c
index 5be51de9f..4ceb57cd9 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -636,7 +636,7 @@ handle_movecursor(
}
}
- if (is_current)
+ if (is_current && visible)
update_cursor();
return 1;
```
We should store the property for the cursor visibility.
I'll merge it in.
…--
hundred-and-one symptoms of being an internet addict:
246. You use up your free 1 Gbyte in two days.
/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
|
|
Yasuhiro Matsumoto wrote:
-- Commit Summary --
* avoid heavy redrawing cursor
-- File Changes --
M src/terminal.c (19)
Thanks. I found one place a flush is still needed, otherwise the screen
is not updated after ":term ls" ends.
…--
hundred-and-one symptoms of being an internet addict:
247. You use www.switchboard.com instead of dialing 411 and 555-12-12
for directory assistance.
/// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
|
Problem: In a terminal the cursor is updated too often.
Solution: Only flush when needed. (Yasuhiro Matsumoto). Remeber whether the
cursor is visible. (closes vim#1873)
No description provided.