From fcd989734235c25e64f8fb38ffcfdf381e933873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Wed, 14 Jul 2021 19:17:44 +0200 Subject: [PATCH] csi: invert the meaning of DECSDM There has been some confusion whether enabling DECSDM (private mode 80) enables or disables sixel scrolling. Foot currently enables scrolling when DECSDM is set, and this patch changes this, such that setting DECSDM now *disables* scrolling. The confusion is apparently due to a documentation error in the VT340 manual, as described in https://github.com/dankamongmen/notcurses/issues/1782#issuecomment-863603641. And that makes sense, in a way: the SDM in DECSDM stands for Sixel Display Mode. I.e. it stands to reason that enabling that disables scrolling. Anyway, this lead to https://github.com/hackerb9/lsix/issues/41, where it was eventually proven (by testing on a real VT340), that foot, and a large number of other terminals (including XTerm) has it wrong: https://github.com/hackerb9/lsix/issues/41#issuecomment-873269599. --- CHANGELOG.md | 2 ++ csi.c | 8 ++++---- terminal.h | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7bd40d8..20c52d1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ * Non-empty lines are now considered to have a hard linebreak, _unless_ an actual word-wrap is inserted. +* Setting `DECSDM` now _disables_ sixel scrolling, while resetting it + _enables_ scrolling (https://codeberg.org/dnkl/foot/issues/631). ### Deprecated diff --git a/csi.c b/csi.c index fcc2800f..80ee75cd 100644 --- a/csi.c +++ b/csi.c @@ -402,7 +402,7 @@ decset_decrst(struct terminal *term, unsigned param, bool enable) break; case 80: - term->sixel.scrolling = enable; + term->sixel.scrolling = !enable; break; case 1000: @@ -611,7 +611,7 @@ decrqm(const struct terminal *term, unsigned param, bool *enabled) case 12: *enabled = term->cursor_blink.decset; return true; case 25: *enabled = !term->hide_cursor; return true; case 45: *enabled = term->reverse_wrap; return true; - case 80: *enabled = term->sixel.scrolling; return true; + case 80: *enabled = !term->sixel.scrolling; return true; case 1000: *enabled = term->mouse_tracking == MOUSE_CLICK; return true; case 1001: *enabled = false; return true; case 1002: *enabled = term->mouse_tracking == MOUSE_DRAG; return true; @@ -654,7 +654,7 @@ xtsave(struct terminal *term, unsigned param) case 25: term->xtsave.show_cursor = !term->hide_cursor; break; case 45: term->xtsave.reverse_wrap = term->reverse_wrap; break; case 47: term->xtsave.alt_screen = term->grid == &term->alt; break; - case 80: term->xtsave.sixel_scrolling = term->sixel.scrolling; break; + case 80: term->xtsave.sixel_display_mode = !term->sixel.scrolling; break; case 1000: term->xtsave.mouse_click = term->mouse_tracking == MOUSE_CLICK; break; case 1001: break; case 1002: term->xtsave.mouse_drag = term->mouse_tracking == MOUSE_DRAG; break; @@ -696,7 +696,7 @@ xtrestore(struct terminal *term, unsigned param) case 25: enable = term->xtsave.show_cursor; break; case 45: enable = term->xtsave.reverse_wrap; break; case 47: enable = term->xtsave.alt_screen; break; - case 80: enable = term->xtsave.sixel_scrolling; break; + case 80: enable = term->xtsave.sixel_display_mode; break; case 1000: enable = term->xtsave.mouse_click; break; case 1001: return; case 1002: enable = term->xtsave.mouse_drag; break; diff --git a/terminal.h b/terminal.h index 309f7db6..12a637a0 100644 --- a/terminal.h +++ b/terminal.h @@ -378,7 +378,7 @@ struct terminal { bool ime:1; bool app_sync_updates:1; - bool sixel_scrolling:1; + bool sixel_display_mode:1; bool sixel_private_palette:1; bool sixel_cursor_right_of_graphics:1; } xtsave;