[vte] sixel: Use special coulor registers for default foreground and background



commit fdf39568ab0ce287a091db090734886560499462
Author: Christian Persch <chpe gnome org>
Date:   Sat Nov 14 11:51:17 2020 +0100

    sixel: Use special coulor registers for default foreground and background
    
    ... and implement background colour transparency using the second
    parameter to DECSIXEL.
    
    It's not clear from the available documentation of how the 2nd parameter
    to DECSIXEL *should* work, but based on discussion in issue #253, this patch
    makes vte use the value "1" as meaning uninked pixels should be transparent,
    and any other value (including the default) to mean that uninked pixels
    should use the current SGR background colour.
    
    This also fixes an issue where the user-addressable colour register 0 was
    being overwritten with the current SGR foreground colour.
    
    Based on patches by Hans Petter Jansson <hpj hpjansson org>.
    
    https://gitlab.gnome.org/GNOME/vte/-/issues/253

 src/sixel-context.cc | 58 +++++++++++++++++++++++++++-------------------------
 src/sixel-context.hh | 13 ++++++------
 src/sixel-test.cc    | 24 ++++++++++++++--------
 src/vte.cc           | 13 ++++++++++++
 src/vteinternal.hh   |  6 ++++++
 src/vteseq.cc        | 28 ++++++++++++++++++-------
 6 files changed, 93 insertions(+), 49 deletions(-)
---
diff --git a/src/sixel-context.cc b/src/sixel-context.cc
index 7bfaeebe..d196ce6d 100644
--- a/src/sixel-context.cc
+++ b/src/sixel-context.cc
@@ -245,22 +245,22 @@ Context::reset_colors() noexcept
          * Colours 9..14 (name marked with '*') are less saturated
          * versions of colours 1..6.
          */
-        m_colors[0 + 1]  = make_color_rgb( 0,  0,  0); /* HLS(  0,  0,  0) */ /* Black    */
-        m_colors[1 + 1]  = make_color_rgb(20, 20, 80); /* HLS(  0, 50, 60) */ /* Blue     */
-        m_colors[2 + 1]  = make_color_rgb(80, 13, 13); /* HLS(120, 46, 72) */ /* Red      */
-        m_colors[3 + 1]  = make_color_rgb(20, 80, 20); /* HLS(240, 50, 60) */ /* Green    */
-        m_colors[4 + 1]  = make_color_rgb(80, 20, 80); /* HLS( 60, 50, 60) */ /* Magenta  */
-        m_colors[5 + 1]  = make_color_rgb(20, 80, 80); /* HLS(300, 50, 60) */ /* Cyan     */
-        m_colors[6 + 1]  = make_color_rgb(80, 80, 20); /* HLS(180, 50, 60) */ /* Yellow   */
-        m_colors[7 + 1]  = make_color_rgb(53, 53, 53); /* HLS(  0, 53,  0) */ /* Grey 50% */
-        m_colors[8 + 1]  = make_color_rgb(26, 26, 26); /* HLS(  0, 26,  0) */ /* Grey 25% */
-        m_colors[9 + 1]  = make_color_rgb(33, 33, 60); /* HLS(  0, 46, 29) */ /* Blue*    */
-        m_colors[10 + 1] = make_color_rgb(60, 26, 26); /* HLS(120, 43, 39) */ /* Red*     */
-        m_colors[11 + 1] = make_color_rgb(33, 60, 33); /* HLS(240, 46, 29) */ /* Green*   */
-        m_colors[12 + 1] = make_color_rgb(60, 33, 60); /* HLS( 60, 46, 29) */ /* Magenta* */
-        m_colors[13 + 1] = make_color_rgb(33, 60, 60); /* HLS(300, 46, 29) */ /* Cyan*    */
-        m_colors[14 + 1] = make_color_rgb(60, 60, 33); /* HLS(180, 46, 29) */ /* Yellow*  */
-        m_colors[15 + 1] = make_color_rgb(80, 80, 80); /* HLS(  0, 80,  0) */ /* Grey 75% */
+        m_colors[0 + 2]  = make_color_rgb( 0,  0,  0); /* HLS(  0,  0,  0) */ /* Black    */
+        m_colors[1 + 2]  = make_color_rgb(20, 20, 80); /* HLS(  0, 50, 60) */ /* Blue     */
+        m_colors[2 + 2]  = make_color_rgb(80, 13, 13); /* HLS(120, 46, 72) */ /* Red      */
+        m_colors[3 + 2]  = make_color_rgb(20, 80, 20); /* HLS(240, 50, 60) */ /* Green    */
+        m_colors[4 + 2]  = make_color_rgb(80, 20, 80); /* HLS( 60, 50, 60) */ /* Magenta  */
+        m_colors[5 + 2]  = make_color_rgb(20, 80, 80); /* HLS(300, 50, 60) */ /* Cyan     */
+        m_colors[6 + 2]  = make_color_rgb(80, 80, 20); /* HLS(180, 50, 60) */ /* Yellow   */
+        m_colors[7 + 2]  = make_color_rgb(53, 53, 53); /* HLS(  0, 53,  0) */ /* Grey 50% */
+        m_colors[8 + 2]  = make_color_rgb(26, 26, 26); /* HLS(  0, 26,  0) */ /* Grey 25% */
+        m_colors[9 + 2]  = make_color_rgb(33, 33, 60); /* HLS(  0, 46, 29) */ /* Blue*    */
+        m_colors[10 + 2] = make_color_rgb(60, 26, 26); /* HLS(120, 43, 39) */ /* Red*     */
+        m_colors[11 + 2] = make_color_rgb(33, 60, 33); /* HLS(240, 46, 29) */ /* Green*   */
+        m_colors[12 + 2] = make_color_rgb(60, 33, 60); /* HLS( 60, 46, 29) */ /* Magenta* */
+        m_colors[13 + 2] = make_color_rgb(33, 60, 60); /* HLS(300, 46, 29) */ /* Cyan*    */
+        m_colors[14 + 2] = make_color_rgb(60, 60, 33); /* HLS(180, 46, 29) */ /* Yellow*  */
+        m_colors[15 + 2] = make_color_rgb(80, 80, 80); /* HLS(  0, 80,  0) */ /* Grey 75% */
 
         /* Devices may use the same colour palette for DECSIXEL as for
          * text mode, so initialise colours 16..255 to the standard 256-colour
@@ -285,14 +285,14 @@ Context::reset_colors() noexcept
         };
 
         for (auto n = 0; n < 216; ++n)
-                m_colors[n + 16 + 1] = make_cube_color(n / 36, (n / 6) % 6, n % 6);
+                m_colors[n + 16 + 2] = make_cube_color(n / 36, (n / 6) % 6, n % 6);
 
         /* 24-colour greyscale ramp */
         for (auto n = 0; n < 24; ++n)
-                m_colors[n + 16 + 216 + 1] = make_color(8 + n * 10, 8 + n * 10, 8 + n * 10);
+                m_colors[n + 16 + 216 + 2] = make_color(8 + n * 10, 8 + n * 10, 8 + n * 10);
 
         /* Set all other colours to black */
-        for (auto n = 256 + 1; n < k_num_colors + 1; ++n)
+        for (auto n = 256 + 2; n < k_num_colors + 2; ++n)
                 m_colors[n] = make_color(0, 0, 0);
 }
 
@@ -304,6 +304,7 @@ Context::prepare(uint32_t introducer,
                  unsigned bg_red,
                  unsigned bg_green,
                  unsigned bg_blue,
+                 bool bg_transparent,
                  bool private_color_registers,
                  double pixel_aspect) noexcept
 {
@@ -315,20 +316,21 @@ Context::prepare(uint32_t introducer,
         if (private_color_registers)
                 reset_colors();
 
-        /* FIXMEchpe: this all seems bogus. */
-        set_color(0, make_color(bg_red, bg_green, bg_blue));
-        if (private_color_registers)
-                set_color(param_to_color_register(0),
-                          make_color(fg_red, fg_green, fg_blue));
+        if (bg_transparent)
+                m_colors[0] = 0u; /* fully transparent */
+        else
+                m_colors[0] = make_color(bg_red, bg_green, bg_blue);
+
+        m_colors[1] = make_color(fg_red, fg_green, fg_blue);
 
         /*
          * DEC PPLV2 says that on entering DECSIXEL mode, the active colour
-         * is set to colour to colour register 0.
-         * Xterm defaults to register 3.
+         * is set to colour register 0. Xterm defaults to register 3.
+         * We use the current foreground color in our special register 1.
          */
-        set_current_color(param_to_color_register(0));
+        set_current_color(1);
 
-        /* Clear bufer, and scanline offsets */
+        /* Clear buffer and scanline offsets */
         std::memset(m_scanlines_offsets, 0, sizeof(m_scanlines_offsets));
 
         if (m_scanlines_data)
diff --git a/src/sixel-context.hh b/src/sixel-context.hh
index 8364702f..fa7d13e2 100644
--- a/src/sixel-context.hh
+++ b/src/sixel-context.hh
@@ -106,7 +106,7 @@ public:
 
 private:
 
-        color_t m_colors[1 + k_num_colors];
+        color_t m_colors[2 + k_num_colors];
         bool m_palette_modified{false};
 
         color_index_t m_current_color{0};
@@ -272,12 +272,12 @@ private:
         {
                 /* Colour registers are wrapped, as per DEC documentation.
                  *
-                 * We internally reserve a register for fully transparent
-                 * colour, and use register 0 for it since that makes it easier
-                 * to initialise the buffer. Therefore the user-provided
-                 * registers are stored at + 1 their public number.
+                 * We internally reserve registers 0 and 1 for the background
+                 * and foreground colors, the buffer being initialized to 0.
+                 * Therefore the user-provided registers are stored at + 2 their
+                 * public number.
                  */
-                return (param & (k_num_colors - 1)) + 1;
+                return (param & (k_num_colors - 1)) + 2;
         }
 
         inline constexpr color_t
@@ -627,6 +627,7 @@ public:
                      unsigned bg_red,
                      unsigned bg_green,
                      unsigned bg_blue,
+                     bool bg_transparent,
                      bool private_color_registers,
                      double pixel_aspect = 1.0) noexcept;
 
diff --git a/src/sixel-test.cc b/src/sixel-test.cc
index e91c2cd7..d1e0f1db 100644
--- a/src/sixel-test.cc
+++ b/src/sixel-test.cc
@@ -40,6 +40,12 @@ using ParseStatus = vte::sixel::Parser::ParseStatus;
 
 // Parser tests
 
+static inline constexpr auto
+param_to_color_register(unsigned reg)
+{
+        return reg + 2; /* Public colour registers start at 2 */
+}
+
 static char const*
 cmd_to_str(Command command)
 {
@@ -1026,6 +1032,7 @@ parse_image(C& context,
         context.prepare(0x50 /* C0 DCS */,
                         fg_red, fg_green, fg_blue,
                         bg_red, bg_green, bg_blue,
+                        false /* bg transparent */,
                         private_color_registers);
 
         auto str_st = std::string{str};
@@ -1050,7 +1057,8 @@ parse_image(C& context,
         parse_image(context, ItemStringifier(items).string(),
                     fg_red, fg_green, fg_blue,
                     bg_red, bg_green, bg_blue,
-                    private_color_registers, line);
+                    private_color_registers,
+                    line);
 }
 
 template<class C>
@@ -1442,7 +1450,7 @@ test_context_image_stride(void)
         assert_image_dimensions(context, 2, 12);
 
         auto data = pixels.get();
-        auto const reg = 1 + 1; /* Colour registers start at 1 */
+        auto const reg = param_to_color_register(1);
 
         for (auto y = 0u; y < context.image_height(); ++y) {
                 for (auto x = 0u; x < context.image_width(); ++x)
@@ -1490,10 +1498,10 @@ test_context_image_palette(void)
                                      unsigned b) constexpr noexcept -> Context::color_t
                 {
                         if constexpr (std::endian::native == std::endian::little) {
-                                        return b | g << 8 | r << 16 | 0xffu << 24 /* opaque */;
-                                } else if constexpr (std::endian::native == std::endian::big) {
-                                        return 0xffu /* opaque */ | r << 8 | g << 16 | b << 24;
-                                } else {
+                                return b | g << 8 | r << 16 | 0xffu << 24 /* opaque */;
+                        } else if constexpr (std::endian::native == std::endian::big) {
+                                return 0xffu /* opaque */ | r << 8 | g << 16 | b << 24;
+                        } else {
                                 __builtin_unreachable();
                         }
                 };
@@ -1521,7 +1529,7 @@ test_context_image_palette(void)
         for (auto n = 0; n < context.num_colors(); ++n) {
                 g_assert_cmpuint(make_color_rgb(palette[n].r, palette[n].g, palette[n].b),
                                  ==,
-                                 context.color(n + 1));
+                                 context.color(param_to_color_register(n)));
         }
 }
 
@@ -1537,7 +1545,7 @@ test_context_image_compositing(void)
 
         auto data = pixels.get();
         for (auto y = 0u; y < context.image_height(); ++y) {
-                auto const reg = (256 + y / 3) + 1; /* registers start at 1 */
+                auto const reg = param_to_color_register((256 + y / 3));
                 for (auto x = 0u; x < context.image_width(); ++x)
                         g_assert_cmpuint(*data++, ==, reg);
         }
diff --git a/src/vte.cc b/src/vte.cc
index 293e463d..deb64fc7 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -8070,6 +8070,19 @@ Terminal::determine_cursor_colors(VteCell const* cell,
                          fore, back, deco);
 }
 
+void
+Terminal::resolve_normal_colors(VteCell const* cell,
+                                unsigned* pfore,
+                                unsigned* pback,
+                                vte::color::rgb& fg,
+                                vte::color::rgb& bg)
+{
+        auto deco = unsigned{};
+        determine_colors(cell, false, pfore, pback, &deco);
+        rgb_from_index<8, 8, 8>(*pfore, fg);
+        rgb_from_index<8, 8, 8>(*pback, bg);
+}
+
 // FIXMEchpe this constantly removes and reschedules the timer. improve this!
 bool
 Terminal::text_blink_timer_callback()
diff --git a/src/vteinternal.hh b/src/vteinternal.hh
index ac7c8654..19b8c19f 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -1010,6 +1010,12 @@ public:
                                             guint *pback,
                                             guint *pdeco) const;
 
+        void resolve_normal_colors(VteCell const* cell,
+                                   unsigned* pfore,
+                                   unsigned* pback,
+                                   vte::color::rgb& fg,
+                                   vte::color::rgb& bg);
+
         char *cellattr_to_html(VteCellAttr const* attr,
                                char const* text) const;
         VteCellAttr const* char_to_cell_attr(VteCharAttributes const* attr) const;
diff --git a/src/vteseq.cc b/src/vteseq.cc
index 312bd10d..88869c13 100644
--- a/src/vteseq.cc
+++ b/src/vteseq.cc
@@ -4382,8 +4382,8 @@ Terminal::DECSIXEL(vte::parser::Sequence const& seq)
          *
          * Defaults:
          *   args[0]: 0
-         *   args[0]: 2 (1 for printers)
-         *   args[0]: no default
+         *   args[1]: 2 (1 for printers)
+         *   args[2]: no default
          *
          * References: VT330
          *             DEC PPLV2 ยง 5.4
@@ -4437,16 +4437,30 @@ Terminal::DECSIXEL(vte::parser::Sequence const& seq)
                 return false;
         }
 
+        /* How to interpret args[1] is not entirely clear from the DEC
+         * documentation and other terminal emulators.
+         * We choose to make args[1]==1 mean to use transparent background.
+         * and treat all other values (default, 0, 2) as using the current
+         * SGR background colour. See the discussion in issue #253.
+         *
+         * Also use the current SGR foreground colour to initialise
+         * the special colour register so that SIXEL images which set
+         * no colours get a sensible default.
+         */
+        auto const transparent_bg = seq.collect1(1, 2) == 1;
+
+        auto fore = unsigned{}, back = unsigned{};
+        auto fg = vte::color::rgb{}, bg = vte::color::rgb{};
+        resolve_normal_colors(&m_color_defaults, &fore, &back, fg, bg);
+
         try {
                 if (!m_sixel_context)
                         m_sixel_context = std::make_unique<vte::sixel::Context>();
 
-                auto const fg = get_color(VTE_DEFAULT_FG);
-                auto const bg = get_color(VTE_DEFAULT_BG);
-
                 m_sixel_context->prepare(seq.st(),
-                                         fg->red >> 8, fg->green >> 8, fg->blue >> 8,
-                                         bg->red >> 8, bg->green >> 8, bg->blue >> 8,
+                                         fg.red >> 8, fg.green >> 8, fg.blue >> 8,
+                                         bg.red >> 8, bg.green >> 8, bg.blue >> 8,
+                                         back == VTE_DEFAULT_BG || transparent_bg,
                                          m_modes_private.XTERM_SIXEL_PRIVATE_COLOR_REGISTERS());
 
                 m_sixel_context->set_mode(mode);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]