Christian Brabandt wrote:
> I am maintaining a eye-candy statusline plugin, that can also create a
> nice tabline. I recently made a change, trying to improve the
> performance of the plugin by caching certain values
> (https://github.com/vim-airline/vim-airline/commit/b2f301f73c168104cf2202ac5f2e2b7c078c7aa1)
>
> The issue can be reproduced with this little vim snippet:
> ,----
> | function! TabTitle(n)
> | let title = gettabvar(a:n, 'title')
> | if empty(title)
> | " call an expensive function to get the title
> | " try to avoid the expensive call, by caching the title
> | let title='foobar'
> | endif
> | "if a:n == tabpagenr()
> | " let t:title = title
> | "endif
> | call settabvar(a:n, 'title', title)
> | return title
> | endfunction
> |
> | function! DrawTabline()
> | let tabline = ''
> | for i in range(1, tabpagenr('$'))
> | let tabline .= "%(Tab: ".i.' "%{TabTitle('.i.')}"%)'
> | endfor
> | return tabline
> | endfunction
> | set tabline=%!DrawTabline()
> `----
Hmm, switching tabs halfway updating the display is something that
should be avoided.
> However, it turns out, that this causes flicker on movement and so makes
> Vim redraw a lot more often than actually needed. I looked into the
> source, trying to find out, why this happens and I think it happens
> because settabvar does internally switch tabpages (e.g. calling
> enter_tabpage) which unconditionally set must_redraw=CLEAR.
>
> However this happens on a call to draw_tabline() which is done when we
> already updating the screen (e.g. in a callchain coming from
> update_scree()). So I think we can prevent this by only setting
> must_redraw, if we are not updating the screen currently, which is what
> this patch does:
First of all, if we don't actually switch tab pages, then
enter_tabpage() won't be called. If we do change tab pages, I don't
know what the effects are. At least it doesn't trigger autocommands, so
perhaps it's not so bad.
> diff --git a/src/window.c b/src/window.c
> index 8078e011d..785d240e8 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -3925,7 +3925,8 @@ enter_tabpage(
>
> last_status(FALSE); /* status line may appear or
> disappear */
> (void)win_comp_pos(); /* recompute w_winrow for all windows */
> - must_redraw = CLEAR; /* need to redraw everything */
> + if (!updating_screen)
> + must_redraw = CLEAR; /* need to redraw everything */
> #ifdef FEAT_DIFF
> diff_need_scrollbind = TRUE;
> #endif
>
> On a second thought, it might be better not to set must_redraw for
> settabvar() at all, since after we are finished, we switch back the
> tabpage, and thus theoretically a redraw should not be necessary. So
> perhaps this patch is preferable:
Yeah, not setting must_redraw there has unpredictable effects.
> diff --git a/src/evalfunc.c b/src/evalfunc.c
> index 2d796f7e1..4b8f95e50 100644
> --- a/src/evalfunc.c
> +++ b/src/evalfunc.c
> @@ -10385,6 +10385,7 @@ f_settabvar(typval_T *argvars, typval_T *rettv)
> #endif
> char_u *varname, *tabvarname;
> typval_T *varp;
> + int redraw;
>
> rettv->vval.v_number = 0;
>
> @@ -10404,6 +10405,7 @@ f_settabvar(typval_T *argvars, typval_T *rettv)
> )
> {
> #ifdef FEAT_WINDOWS
> + redraw = must_redraw;
> save_curtab = curtab;
> goto_tabpage_tp(tp, FALSE, FALSE);
> #endif
> @@ -10421,6 +10423,7 @@ f_settabvar(typval_T *argvars, typval_T *rettv)
> /* Restore current tabpage */
> if (valid_tabpage(save_curtab))
> goto_tabpage_tp(save_curtab, FALSE, FALSE);
> + must_redraw = redraw; /* avoid an unnecessary redraw */
> #endif
> }
> }
The problem here is that we use a very blunt high-level function just to
be able to access a variable. I don't think there are ever side
effects. The only part that really matters is getting the hash table
for the variables, which happens in find_var_ht().
It seems that refactoring set_var(), splitting off the part that calls
find_var_ht() to use the right tabpage, would avoid a lot of trouble.
> And finally, I made another observation, that looks suspicious as well.
> When update_screen() is called, it will eventually call this code, if
> type==CLEAR:
> ,----
> | if (type == CLEAR) /* first clear screen */
> | {
> | screenclear(); /* will reset clear_cmdline */
> | type = NOT_VALID;
> | }
> `----
>
> screenclear() will then call screenclear2() which will call
> win_rest_invalid(firstwin)
> which will then call
> ,----
> | redraw_win_later(wp, NOT_VALID);
> `----
>
> So although we are in the process of updating the current screen, we
> will force another redraw after the next main_loop. I think we can save
> that call here, and only call redraw_win_later if updating_screen is not
> true.
Nice catch. Happens when typing CTL-L in any window but the first one.
Resetting must_redraw is probably the best solution. If for some reason
the screen is cleared when halfway redrawing, setting the flag is
correct. The win->w_redr_type flags will be reset after the window has
been redrawn.
--
>From "know your smileys":
(X0||) Double hamburger with lettuce and tomato
/// Bram Moolenaar -- [email protected] -- 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 ///
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
