RMichelsen / Nvy

Nvy - A Neovim client in C++
MIT License
333 stars 28 forks source link

Improve Unicode support #97

Closed Zorbn closed 1 year ago

Zorbn commented 1 year ago

Previously Unicode support was pretty limited and tied to wide glyphs. It was easy to break Unicode rendering by editing characters around Unicode glyphs. This PR fixes those problems and makes Unicode glyphs behave as expected (like Neovim Qt, Windows Terminal, etc).

The main change is that surrogate pairs get packed into a single cell, since there are many surrogate pairs that only represent a single cell of space. This means that the grid is now made up of uint32s instead of wchars, and the grid chars are unpacked into a buffer of wchars as needed (mainly for interfacing with DirectWrite).

Zorbn commented 1 year ago

This should fix #88.

RMichelsen commented 1 year ago

Thank you for the work on this. From my quick testing I tried opening the contents of this page https://www.cogsci.ed.ac.uk/~richard/unicode-sample.html and I hit an assertion on https://github.com/RMichelsen/Nvy/blob/aeea2eae65e9b2a98996bb2879b558f88a4c9259/src/renderer/renderer.cpp#L726 (we also hit this assertion on the master branch, so its not a regression). Removing the assertion we get to keep running but I still get rendering artifacts and wrong cursor placement etc. I am not sure if this is because these particular unicode chars are troublesome or what the matter is. Anyway, I'd be happy to merge it if the assertion is removed/fixed and these changes bring value to some people.

Zorbn commented 1 year ago

There were two problems, I forgot to change the memcpy call used for scrolling which caused the bulk of the rendering issues. I also found that the surrogate pair handling breaks when rendering diacritics, so instead I changed it to handle that case more gracefully and just render a box.

All that is in the most recent commit. I'll look more into rendering the diacritics properly when I get the chance.

Zorbn commented 1 year ago

I'm trying to find an example of working diacritic rendering to see how it's supposed to look but Neovim Qt, Windows Terminal, and Alacritty all break in one way or another. By comparison rendering diacritics as boxes looks surprisingly stable ;)

RMichelsen commented 1 year ago

From my testing this is good to go, and should certainly break less times than the current solution. Merging :)