From 3d2588edf8a90a7ba74d75089193a52b69905a4f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= <daniel@ekloef.se>
Date: Mon, 15 Apr 2024 16:07:47 +0200
Subject: [PATCH] sixel: don't allow pan/pad changes after sixel data has been
 emitted

Changing pan/pad changes the sixel's aspect ratio. While I don't know
for certain what a real VT340 would do, I suspect it would change the
aspect ratio of all subsequent sixels, but not those already emitted.

The way we implement sixels in foot, makes this behavior hard to
implement. We currently don't resize the image properly if the aspect
ratio is changed, but not the RA area. We have code that assumes all
sixel lines have the same aspect ratio, etc.

Since no "normal" applications change the aspect ratio in the middle
of a sixel, simply disallow it, and print a warning.

This also fixes a crash, when writing sixels after having modified the
aspect ratio.
---
 CHANGELOG.md |  3 +++
 sixel.c      | 26 +++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cad31efd..f4276f0a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -62,6 +62,9 @@
 ### Removed
 ### Fixed
 
+* Crash when changing aspect ratio of a sixel, in the middle of the
+  sixel data (this is unsupported in foot, but should of course not
+  result in a crash).
 * Crash when printing double-width (or longer) characters to, or near,
   the last column, when auto-wrap (private mode 7) has been disabled.
 * Dynamically sized sixel being trimmed to nothing.
diff --git a/sixel.c b/sixel.c
index 85be3608..d6bfa3a4 100644
--- a/sixel.c
+++ b/sixel.c
@@ -1852,12 +1852,32 @@ decgra(struct terminal *term, uint8_t c)
         pan = pan > 0 ? pan : 1;
         pad = pad > 0 ? pad : 1;
 
+        if (likely(term->sixel.image.width == 0 &&
+                   term->sixel.image.height == 0))
+        {
+            term->sixel.pan = pan;
+            term->sixel.pad = pad;
+        } else {
+            /*
+             * Unsure what the VT340 does...
+             *
+             * We currently do *not* handle changing pan/pad in the
+             * middle of a sixel, since that means resizing/stretching
+             * the existing image.
+             *
+             * I'm *guessing* the VT340 simply changes the aspect
+             * ratio of all subsequent sixels. But, given the design
+             * of our implementation (the entire sixel is written to a
+             * single pixman image), we can't easily do that.
+             */
+            LOG_WARN("sixel: unsupported: pan/pad changed after printing sixels");
+            pan = term->sixel.pan;
+            pad = term->sixel.pad;
+        }
+
         pv *= pan;
         ph *= pad;
 
-        term->sixel.pan = pan;
-        term->sixel.pad = pad;
-
         LOG_DBG("pan=%u, pad=%u (aspect ratio = %d:%d), size=%ux%u",
                 pan, pad, pan, pad, ph, pv);