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



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

    Revert "sixel: Use special coulor registers for default foreground and background"
    
    This reverts commit 65234343b88b12072ff8414ce9a6742ddbade407.
    
    There is nothing technically wrong with this commit, but it was a heavily amended
    commit based on squashing two commits by the Author, and I forgot to change the Author
    to myself. The next commit will re-commit this with proper attribution.

 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, 49 insertions(+), 93 deletions(-)
---
diff --git a/src/sixel-context.cc b/src/sixel-context.cc
index d196ce6d..7bfaeebe 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 + 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% */
+        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% */
 
         /* 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 + 2] = make_cube_color(n / 36, (n / 6) % 6, n % 6);
+                m_colors[n + 16 + 1] = 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 + 2] = make_color(8 + n * 10, 8 + n * 10, 8 + n * 10);
+                m_colors[n + 16 + 216 + 1] = make_color(8 + n * 10, 8 + n * 10, 8 + n * 10);
 
         /* Set all other colours to black */
-        for (auto n = 256 + 2; n < k_num_colors + 2; ++n)
+        for (auto n = 256 + 1; n < k_num_colors + 1; ++n)
                 m_colors[n] = make_color(0, 0, 0);
 }
 
@@ -304,7 +304,6 @@ 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
 {
@@ -316,21 +315,20 @@ Context::prepare(uint32_t introducer,
         if (private_color_registers)
                 reset_colors();
 
-        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);
+        /* 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));
 
         /*
          * DEC PPLV2 says that on entering DECSIXEL mode, the active colour
-         * is set to colour register 0. Xterm defaults to register 3.
-         * We use the current foreground color in our special register 1.
+         * is set to colour to colour register 0.
+         * Xterm defaults to register 3.
          */
-        set_current_color(1);
+        set_current_color(param_to_color_register(0));
 
-        /* Clear buffer and scanline offsets */
+        /* Clear bufer, 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 fa7d13e2..8364702f 100644
--- a/src/sixel-context.hh
+++ b/src/sixel-context.hh
@@ -106,7 +106,7 @@ public:
 
 private:
 
-        color_t m_colors[2 + k_num_colors];
+        color_t m_colors[1 + 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 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.
+                 * 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.
                  */
-                return (param & (k_num_colors - 1)) + 2;
+                return (param & (k_num_colors - 1)) + 1;
         }
 
         inline constexpr color_t
@@ -627,7 +627,6 @@ 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 d1e0f1db..e91c2cd7 100644
--- a/src/sixel-test.cc
+++ b/src/sixel-test.cc
@@ -40,12 +40,6 @@ 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)
 {
@@ -1032,7 +1026,6 @@ 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};
@@ -1057,8 +1050,7 @@ 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>
@@ -1450,7 +1442,7 @@ test_context_image_stride(void)
         assert_image_dimensions(context, 2, 12);
 
         auto data = pixels.get();
-        auto const reg = param_to_color_register(1);
+        auto const reg = 1 + 1; /* Colour registers start at 1 */
 
         for (auto y = 0u; y < context.image_height(); ++y) {
                 for (auto x = 0u; x < context.image_width(); ++x)
@@ -1498,10 +1490,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();
                         }
                 };
@@ -1529,7 +1521,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(param_to_color_register(n)));
+                                 context.color(n + 1));
         }
 }
 
@@ -1545,7 +1537,7 @@ test_context_image_compositing(void)
 
         auto data = pixels.get();
         for (auto y = 0u; y < context.image_height(); ++y) {
-                auto const reg = param_to_color_register((256 + y / 3));
+                auto const reg = (256 + y / 3) + 1; /* registers start at 1 */
                 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 deb64fc7..293e463d 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -8070,19 +8070,6 @@ 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 19b8c19f..ac7c8654 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -1010,12 +1010,6 @@ 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 88869c13..312bd10d 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[1]: 2 (1 for printers)
-         *   args[2]: no default
+         *   args[0]: 2 (1 for printers)
+         *   args[0]: no default
          *
          * References: VT330
          *             DEC PPLV2 ยง 5.4
@@ -4437,30 +4437,16 @@ 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,
-                                         back == VTE_DEFAULT_BG || transparent_bg,
+                                         fg->red >> 8, fg->green >> 8, fg->blue >> 8,
+                                         bg->red >> 8, bg->green >> 8, bg->blue >> 8,
                                          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]