Skip to content

Commit

Permalink
gui_gtk_x11: make bypassing the ASCII glyph cache user-settable
Browse files Browse the repository at this point in the history 
This patch introduces a variable, g:gtk_nocache which (if set) contains
four integers forming a bit map that specifies for which characters <
128 the ASCII glyph cache should be bypassed, and proper glyph shaping
procedures applied instead. If the variable is unset, the glyph cache is
used unconditionally. If set to e.g. [0x00000000, 0xfc00ffff,
0xf8000001, 0x78000001], this will bypass the glyph cache for everything
but control characters (< 32), [0-9A-Za-z], and ASCII code 127. When
using the GUI with a font like Hasklig or PragmataPro, this will enable
most of the programming ligatures (rendering artefacts from
redrawing/cursor movement will still occur, ^L fixes those).

Conflicts:
	src/gui_gtk_x11.c
  • Loading branch information
@manuelschiller
manuelschiller committed Sep 14, 2016
1 parent 789a5c0 commit 0c8d6a7
Showing 1 changed file with 38 additions and 5 deletions.
43 changes: 38 additions & 5 deletions src/gui_gtk_x11.c
View file
Expand Up @@ -5940,11 +5940,44 @@ gui_gtk2_draw_string(int row, int col, char_u *s, int len, int flags)
&& !((flags & DRAW_BOLD) && gui.font_can_bold)
&& gui.ascii_glyphs != NULL)
{
char_u *p;

for (p = s; p < s + len; ++p)
if (*p & 0x80)
goto not_ascii;
/* Check global variable gtk_nocache to get a list of integers
* describing a bit map of ASCII characters (< 128) for which to
* disable the ASCII glyph cache (and hence enable ligatures for fonts
* that support them).
*
* To bypass the glyph cache for anything but control characters,
* space, 0-9, A-Z, a-z, DEL, use:
*
* let g:gtk_nocache=[0x00000000, 0xfc00ffff, 0xf8000001, 0x78000001]
*/
static char_u* vname = (char_u*) "g:gtk_nocache";
dictitem_T *di = find_var(vname, NULL, FALSE);
list_T* l = (NULL == di || VAR_LIST != di->di_tv.v_type) ?
NULL : di->di_tv.vval.v_list;

if (NULL == l || 4 != list_len(l)) {
/* simple case: always go through cache for characters < 128 */
char_u *p, *q;
for (p = s, q = s + len; p < q; ++p)
if (*p & 0x80)
goto not_ascii;
} else {
/* complicated case: get bitmap, check which characters bypass
* the cache */
listitem_T* li = l->lv_first;
guint32 bmap[4];
char_u *p, *q;
bmap[0] = (int) get_tv_number(&li->li_tv);
li = li->li_next;
bmap[1] = (int) get_tv_number(&li->li_tv);
li = li->li_next;
bmap[2] = (int) get_tv_number(&li->li_tv);
li = li->li_next;
bmap[3] = (int) get_tv_number(&li->li_tv);
for (p = s, q = s + len; p < q; ++p)
if (*p & 0x80 || (1 & (bmap[*p >> 5] >> (*p & 31))))
goto not_ascii;
}

pango_glyph_string_set_size(glyphs, len);

Expand Down

19 comments on commit 0c8d6a7

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manuelschiller do you have any idea about vim#418 (comment) … I have been carrying your patch in my COPR, but the commenter is right: your patch makes rendering of characters horribly slow. Any idea, what’s going on? Any idea how to profile it?

@gasparch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manuel, first thank you for your work, it really makes gvim's look more pleasant :)

Giving this patch a second thought, I've got a question if makes sense to move g:gtk_nocache checking and parsing code somewhere else, so that it would not be invoked on every render. I'm not familiar with Vim codebase, but I would imagine it is possible to do on variable set.
If not - then at least parse results may be cached?

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
@mcepl: Well, it depends a bit on the settings - I keep ligatures off for all alphanumeric characters and whitespace. That keeps text rendering reasonably quick. I haven't really looked into this patch since I wrote it, but it seems to work fine on my machines (even the uniprocessor Eee PC running at 570 MHz). The trouble is that the shaping process (i.e. recognising which characters form ligatures, and selecting the appropriate glyph for them) is a lot more work than just having a character-glyph table (which is what vim does without the patch - that's why ligatures aren't working). I'm not sure we can easily implement something that's faster than the shaping code in the library, and remain correct.

@gasparch: Good point - the drawing code should just access a bitmap to see for which characters the built-in character-glyph lookup table should be disabled, and that's it! The parsing code doesn't belong there. Any suggestions as to where to move it? I'm not really familiar with the vim codebase either, so suggestions are welcome.

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 14, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep ligatures off for all alphanumeric characters and whitespace.

@manuelschiller Can you suggest some value of g:gtk_nocache which would allow ligatures for +=, <=, !=, and similar combinations, and it wouldn’t be that computationally demanding? I guess ASCII characters 33-47, 58-63, 91-96, 123, 125, or perhaps I can sacrifice some (especially those last ones).

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcepl I have the following in my .vimrc:

let g:gtk_nocache=[0x00000000, 0xfc00ffff, 0xf8000001, 0x78000001]

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manuelschiller let me try it.

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 14, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manuelschiller Oh, that's the same I have, and it constitutes a significant speed bump here for me. Not too bad when working on the programming code, but significant for longer text (I write also a novel in vim with rST). Would it be possible to make this setting per-buffer (I don’t use ligatures in my prose)?

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcepl Well, could be. Just out of curiosity: Have you confirmed that my patch is the main cause of the slowdown (e.h. by building vim with and without that patch)? In my experience, it's usually the other plugins that I use that make things really slow. Some of my autocompletion plugins for C/C++ code have a much bigger effect than ligatures. Maybe it's the same for you?

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly, there are plugins which are terrible (e.g., otherwise quite useful https://github.com/dbmrq/vim-ditto went straight to opt subdirectory of my vim plugins, because it horribly slows down everything; I am considering rewriting it as an external program called via some job_* methods), but the comparison I meant was done keeping everything else constant. Just by rebuilding gvim without your patch made rewriting of text significantly faster, so much I can feel it.

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, fair enough. I'll have a look when I have a minute, but it may take some time, since I'm travelling for work for the next two weeks. Out of interest, what kind of machine are you working on, and which language are you writing in?

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 14, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I work for Red Hat in the Desktop QA team, so most of my professional work are Python testing scripts. I am also the primary maintainer of M2Crypto, so that’s again mostly Python and a lot of C. So, ligatures I care about are mostly those shown above. No Haskell, Erlang or something similarly crazy (in terms of ligatures). Computer I have is pretty nice Lenovo Thinkpad T440s (dual-processor, quadcore, 11GB RAM, the only drive is SDD, distribution is of course RHEL-7), so I don't think a slow computer is the problem. If vim doesn’t work smoothly on such machine, there is something wrong.

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, that kind of machine ought to be fast enough (just checking, because people use all sorts of machines). I have something pretty similar, and do a lot of staring at code for the LHCb experiment at CERN. I'll see what I can do when I get the slides for the talk next week done.

@gasparch
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcepl - can you please also dump information about exact OS version, gtk versions, pango version?, Xorg version, drivers in use, etc. Because it seems very odd to have slowdown on such fast computer.

Also please tell which font do you use.

It may be slowdown by pango trying to determine width/glyphs.

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally using PragmataPro for serious work, and Iosevka when I'm really tired, or in an odd mood. Both are excellent fonts, the latter has the advantage that it's free.

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RHEL 7.5/Nightly (i.e., not released yet, but roughly comparable to Fedora 26), gtk3-3.22.26-1.el7.x86_64,
pango-1.40.4-1.el7.x86_64, xorg-x11-server-Xorg-1.19.5-2.el7.x86_64 (sometimes trying Wayland, but it is broken right now), intel driver (xorg-x11-drv-intel-2.99.917-26.20160929.el7.x86_64), but using modeset.

Xorg.0.log

:set guifont?
  guifont=Fira Code Medium 11

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcepl Okay, I've had a look, and it seems that I can move the "parsing" and preprocessing out of the rendering loop if I make this an option (as in option.[ch]), but I'll have to do more research and more testing before I can give any details and produce a patch. In any case, it looks like it'll be possible to move some code out of the hot code path (which is always a good thing). I'm still not convinced it accounts for the slowdown you're experiencing, but I won't rule it out for now (since I haven't done any profiling)...

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 16, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Record from perf recorded on gvim with ligatures and the one without ligatures. This is top of the report from perf diff between the two records.

# Event 'cycles:ppp'
#
# Baseline  Delta Abs  Shared Object                  Symbol
# ........  .........  .............................  ............................
#
    60.00%    -24.00%  libc-2.17.so                   [.] _int_malloc
    40.00%    -16.00%  [kernel.kallsyms]              [k] timerqueue_add
     0.00%    +14.43%  libharfbuzz.so.0.10302.0       [.] 0x000000000000b0af
     0.00%     +2.89%  libc-2.17.so                   [.] __memset_sse2
     0.00%     +2.53%  libcairo.so.2.11400.8          [.] clip_and_composite_boxes

Actually, after all, I have probably found one more culprit to my slowness. ALE uses proselint even without my knowing and it is just a pure performance disaster. When removing the thing from my computer there is still difference in performance, but now it is so small, I can happily survive with it.

@manuelschiller
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcepl Okay, that's great to hear. This is more in line with my expectations (given that I regularly run gvim with the patch on an old 570 MHz machine with very little RAM by today's standards, and it works okay) - and also with what I seem to recall from when I wrote the patch, and benchmarked the results.

Still, your suggestion of pulling the code initialising the bitmap for which character to keep or disable the shaping/glyph cache is a good one, and it's one that I've been thinking about for quite some time, but lacked the energy to implement. I think I'll use your complaint as an excuse, and see what I can do in the next few weeks whenever I have a moment (and the energy to do it).

@mcepl
Copy link

@mcepl mcepl commented on 0c8d6a7 Nov 26, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

  19.88%  gvim     libgtk-3.so.0.2200.26          [.] gtk_progress_tracker_get_iteration_cy
  17.54%  gvim     libharfbuzz.so.0.10302.0       [.] hb_buffer_t::next_glyph
  15.06%  gvim     libharfbuzz.so.0.10302.0       [.] apply_forward
   9.35%  gvim     libharfbuzz.so.0.10302.0       [.] apply_string<GSUBProxy>
   4.68%  gvim     libc-2.17.so                   [.] __memset_sse2
   4.53%  gvim     libharfbuzz.so.0.10302.0       [.] hb_prealloced_array_t<hb_get_subtable
   4.40%  gvim     libharfbuzz.so.0.10302.0       [.] OT::SubstLookupSubTable::dispatch<hb_
   2.34%  gvim     gvim                           [.] gui_gtk2_draw_string
   2.34%  gvim     libcairo.so.2.11400.8          [.] composite_glyphs
   1.17%  gvim     libharfbuzz.so.0.10302.0       [.] hb_ot_map_t::substitute
   1.17%  gvim     gvim                           [.] addstate
   1.17%  gvim     libcairo.so.2.11400.8          [.] _cairo_scaled_font_glyph_device_exten
   1.17%  gvim     libpango-1.0.so.0.4000.4       [.] pango_script_iter_next
   1.17%  gvim     libc-2.17.so                   [.] _int_free
   1.17%  gvim     libpangocairo-1.0.so.0.4000.4  [.] _pango_cairo_font_private_get_glyph_e
   1.17%  gvim     libglib-2.0.so.0.5400.2        [.] g_slice_free1
   1.17%  gvim     gvim                           [.] in_id_list
   1.17%  gvim     libcairo.so.2.11400.8          [.] _cairo_scaled_font_glyph_approximate_
   1.17%  gvim     libpango-1.0.so.0.4000.4       [.] width_iter_iswide
   1.17%  gvim     libharfbuzz.so.0.10302.0       [.] hb_font_get_nominal_glyph_trampoline
   1.17%  gvim     libcairo.so.2.11400.8          [.] _cairo_gstate_ensure_scaled_font
   1.17%  gvim     libpango-1.0.so.0.4000.4       [.] pango_font_description_equal
   1.17%  gvim     libpango-1.0.so.0.4000.4       [.] pango_renderer_deactivate
   1.17%  gvim     [kernel.kallsyms]              [k] copy_user_enhanced_fast_string
   1.17%  gvim     libharfbuzz.so.0.10302.0       [.] g_unicode_script_to_iso15924@plt
   1.17%  gvim     libpython2.7.so.1.0            [.] PyDict_Next

I wonder there is some kind of leak or some other kind of degradation. After working on multiple files for couple of hours (none of them bigger than 100K, so size shouldn't be the issue) speed of the writing got progressively slower and slower. Never that bad as with proselint, but quite unpleasantly slow. In that moment I took a snapshot of the performance with perf.

Please sign in to comment.