GNOME Bugzilla – Bug 795192
[PATCH] Fix creating wrong cell array in _vte_ring_thaw_row()
Last modified: 2018-10-19 20:18:48 UTC
When cat nabe.txt, created correct cell array. Correct cell.c 9089 (col = 0) 80000001 (col = 2) decomp->prefix=9089, decomp->suffix=e0110 80000002 (col = 4) 80000003 (col = 6) 80000004 (col = 8) When resize/maximize/minimize window, created wrong cell array. Wrong cell.c 9089 (col = 0) 9089 (col = 2) 9089 (col = 4) 9089 (col = 6) 9089 (col = 8) Sample text nabe.txt has IVS. printf '\U9089' > nabe.txt printf '\U9089\Ue0110' >> nabe.txt printf '\U9089\Ue0111' >> nabe.txt printf '\U9089\Ue0112' >> nabe.txt printf '\U9089\Ue0113' >> nabe.txt OR use echo -en "\U9089\Ue0100" so on. This patch is related with https://bugzilla.gnome.org/show_bug.cgi?id=792636 This patch and pango's patch(792636) make display VS(SVS/IVS) on gnome-terminal.
Created attachment 370835 [details] [review] ring::_vte_ring_thaw_row() Fix-creating-wrong-cell-array
Created attachment 370836 [details] Sample : nabe.txt
Thanks! Just for the record: 1. If the bug is indeed in thaw_row() then every line that has been scrolled out by several lines should also be affected by this bug. 2. When you resize, the way rewrap-on-resize is implemented is that it freezes all the rows, then rewraps, and then thaws the onscreen one (not sure if immediately or on demand), that's why a resize could trigger this issue. I'm planning to do some thorough review and deeper investigation (how come that the original processing of data is different from thaw_row(), they should be identical; are there some other discrepancies too?). [Please allow me a couple of weeks to get back to this.]
Created attachment 370976 [details] ScreenShot nabe.txt
Created attachment 370977 [details] ScreenShot after maximize
Created attachment 370978 [details] ScreenShot after maximize (with this patch)
Thanks for your response. When check to display sample nabe.txt, need font ipamjm.ttf (Font for VS). (Place font file at /usr/share/fonts/ on Fedora, RHEL,,etc.) ipamjm.ttf https://mojikiban.ipa.go.jp/1300.html I attached ScreenShot of vte. After maximize vte window, IVS charactes are changed (U+9089+U+E0110 -> U+9089). # Notice : These vte are compiled with patched pango (https://bugzilla.gnome.org/show_bug.cgi?id=792636) Pango's patch is https://bugzilla.gnome.org/attachment.cgi?id=367056
Created attachment 373885 [details] [review] Fix v2 I totally forgot about this bug, my bad. It came to my attention again via the duplicate https://gitlab.gnome.org/GNOME/vte/issues/59. Akira, your patch mangles prev_columns while applying the combining accent over a double-wide character. So in case of two or more combining accents only the first one is preserved, the remaining ones are lost. I rewrote it to fix it, plus I made the changes more "local", which I hope makes it more robust against further changes (e.g. no risk of forgetting to update prev_columns in a different branch). I'd still like to credit it to you, for your precious investigation and initial fix!
Comment on attachment 373885 [details] [review] Fix v2 + /* Spread it to all the cells of a potentially multicell character */ + for (int i = row->len - 1; i >= 1 && row->cells[i].attr.fragment(); i++) { + row->cells[i - 1].c = row->cells[i].c; Hmm. This doesn't make sense to me? I don't see how this is safe; doesn't this always test the past-the-end cells[row->len] if the last cell (cells[row->len - 1]) is a fragment? Did you mean to use "i--" in the loop maybe?
Oh, sh!t... i-- of course!
Created attachment 373927 [details] [review] Fix v3
Comment on attachment 373927 [details] [review] Fix v3 Looks good now, thanks! Master, and 0-54 (and as far back on older stable branches as you want/care).
Comment on attachment 373927 [details] [review] Fix v3 Cherry-picked to 0-54 and 0-52, just in case Ubuntu will pick it up for 18.04 LTS (they won't). Should apply cleanly to older branches too, if Rishi needs it for RHEL or Fedora.