patch 9.2.0332: popup: still opacity rendering issues

Problem:  popup: still opacity rendering issues
Solution: Fix remaining issues, see below
          (Yasuhiro Matsumoto).

This PR fixes the following issues:

- Padding blend hole at wide char boundary: when a padding cell overlaps
  the second half of a wide character, the right half's attr value is
  unreliable. Use the left half's saved attr for blending instead.

- Wide char background split at popup boundary: when a wide character in
  an upper popup straddles the edge of a lower opacity popup, both
  halves got different background colors. Since terminals cannot render
  different left/right background colors for a wide character, detect
  the lower popup with popup_is_over_opacity() and use the non-popup
  side's underlying attr for both halves.

- Wrong blend color with cterm-only highlights under 'termguicolors':
  when a popup highlight has ctermbg but no guibg, bg_rgb is set to
  CTERMCOLOR (not INVALCOLOR). hl_blend_attr() used this value as a real
  RGB color, producing gray instead of the intended color. Use
  COLOR_INVALID() to detect both INVALCOLOR and CTERMCOLOR, and fall back
  to converting the cterm color number to RGB.

closes: #19943

Signed-off-by: Yasuhiro Matsumoto <mattn.jp@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

src/highlight.c

@@ -3125,6 +3125,32 @@ hl_combine_attr(int char_attr, int prim_attr)
 }
 
 #if defined(FEAT_GUI) || defined(FEAT_TERMGUICOLORS)
+
+# ifdef FEAT_TERMGUICOLORS
+/*
+ * Convert a cterm color number (1-16) to an RGB value.
+ * Used as a fallback when 'termguicolors' is set but only cterm colors are
+ * specified (no guifg/guibg).
+ * Returns INVALCOLOR if the color number is out of range.
+ */
+    static guicolor_T
+cterm_color_to_rgb(int color_nr)
+{
+    // ANSI color order: Black, Red, Green, Yellow, Blue, Magenta,
+    // Cyan, White, then bright variants.
+    static const guicolor_T cterm_color_16[16] = {
+	0x000000, 0xc00000, 0x008000, 0x808000,
+	0x0000c0, 0xc000c0, 0x004080, 0xc0c0c0,
+	0x808080, 0xff8080, 0x00ff00, 0xffff00,
+	0x6060ff, 0xff40ff, 0x00ffff, 0xffffff
+    };
+
+    if (color_nr < 1 || color_nr > 16)
+	return INVALCOLOR;
+    return cterm_color_16[color_nr - 1];
+}
+# endif
+
 /*
  * Blend two RGB colors based on blend value (0-100).
  * blend: 0=use popup color, 100=use background color
@@ -3273,32 +3299,47 @@ hl_blend_attr(int char_attr, int popup_attr, int blend, int blend_fg UNUSED)
 		if (popup_aep->ae_u.cterm.bg_color > 0)
 		    new_en.ae_u.cterm.bg_color = popup_aep->ae_u.cterm.bg_color;
 #ifdef FEAT_TERMGUICOLORS
-		// Blend RGB colors for termguicolors mode
-		if (blend_fg)
+		// Blend RGB colors for termguicolors mode.
+		// Fall back to cterm color converted to RGB when
+		// gui color is not set.
 		{
-		    // blend_fg=TRUE: fade underlying text toward popup bg.
-		    if (popup_aep->ae_u.cterm.bg_rgb != INVALCOLOR)
+		    guicolor_T popup_bg = popup_aep->ae_u.cterm.bg_rgb;
+		    guicolor_T popup_fg = popup_aep->ae_u.cterm.fg_rgb;
+
+		    if (COLOR_INVALID(popup_bg)
+				    && popup_aep->ae_u.cterm.bg_color > 0)
+			popup_bg = cterm_color_to_rgb(
+					popup_aep->ae_u.cterm.bg_color);
+		    if (COLOR_INVALID(popup_fg)
+				    && popup_aep->ae_u.cterm.fg_color > 0)
+			popup_fg = cterm_color_to_rgb(
+					popup_aep->ae_u.cterm.fg_color);
+
+		    if (blend_fg)
 		    {
-			int base_fg = 0xFFFFFF;
-			if (char_aep != NULL
-				&& char_aep->ae_u.cterm.fg_rgb != INVALCOLOR)
-			    base_fg = char_aep->ae_u.cterm.fg_rgb;
-			new_en.ae_u.cterm.fg_rgb = blend_colors(
-				base_fg, popup_aep->ae_u.cterm.bg_rgb, blend);
+			// blend_fg=TRUE: fade underlying text toward popup bg.
+			if (popup_bg != INVALCOLOR)
+			{
+			    int base_fg = 0xFFFFFF;
+			    if (char_aep != NULL
+				    && char_aep->ae_u.cterm.fg_rgb != INVALCOLOR)
+				base_fg = char_aep->ae_u.cterm.fg_rgb;
+			    new_en.ae_u.cterm.fg_rgb = blend_colors(
+				    base_fg, popup_bg, blend);
+			}
+		    }
+		    else if (popup_fg != INVALCOLOR)
+			// blend_fg=FALSE: use popup foreground
+			new_en.ae_u.cterm.fg_rgb = popup_fg;
+		    if (popup_bg != INVALCOLOR)
+		    {
+			// Blend popup bg toward underlying bg
+			guicolor_T underlying_bg = INVALCOLOR;
+			if (char_aep != NULL)
+			    underlying_bg = char_aep->ae_u.cterm.bg_rgb;
+			new_en.ae_u.cterm.bg_rgb = blend_colors(
+				popup_bg, underlying_bg, blend);
 		    }
-		}
-		else if (popup_aep->ae_u.cterm.fg_rgb != INVALCOLOR)
-		    // blend_fg=FALSE: use popup foreground
-		    new_en.ae_u.cterm.fg_rgb = popup_aep->ae_u.cterm.fg_rgb;
-		if (popup_aep->ae_u.cterm.bg_rgb != INVALCOLOR)
-		{
-		    // Blend popup bg toward underlying bg
-		    guicolor_T underlying_bg = INVALCOLOR;
-		    if (char_aep != NULL)
-			underlying_bg = char_aep->ae_u.cterm.bg_rgb;
-		    new_en.ae_u.cterm.bg_rgb = blend_colors(
-			    popup_aep->ae_u.cterm.bg_rgb,
-			    underlying_bg, blend);
 		}
 #endif
 	    }

src/popupwin.c

@@ -4438,6 +4438,37 @@ popup_is_under_opacity(int row, int col)
     return opacity_zindex[row * opacity_zindex_cols + col] > screen_zindex;
 }
 
+/*
+ * Return TRUE if cell (row, col) is covered by a lower-zindex opacity popup.
+ */
+    int
+popup_is_over_opacity(int row, int col)
+{
+    win_T *wp;
+
+    FOR_ALL_POPUPWINS(wp)
+	if ((wp->w_popup_flags & POPF_OPACITY)
+		&& wp->w_popup_blend > 0
+		&& !(wp->w_popup_flags & POPF_HIDDEN)
+		&& wp->w_zindex < screen_zindex
+		&& row >= wp->w_winrow
+		&& row < wp->w_winrow + popup_height(wp)
+		&& col >= wp->w_wincol
+		&& col < wp->w_wincol + popup_width(wp))
+	    return TRUE;
+    FOR_ALL_POPUPWINS_IN_TAB(curtab, wp)
+	if ((wp->w_popup_flags & POPF_OPACITY)
+		&& wp->w_popup_blend > 0
+		&& !(wp->w_popup_flags & POPF_HIDDEN)
+		&& wp->w_zindex < screen_zindex
+		&& row >= wp->w_winrow
+		&& row < wp->w_winrow + popup_height(wp)
+		&& col >= wp->w_wincol
+		&& col < wp->w_wincol + popup_width(wp))
+	    return TRUE;
+    return FALSE;
+}
+
 /*
  * Return TRUE if any cell in row "row" from "start_col" to "end_col"
  * (exclusive) is covered by a higher-zindex opacity popup.
@@ -4840,8 +4871,10 @@ draw_opacity_padding_cell(
 			screen_char(base_off, row, base_col);
 
 			// Draw padding in the right half.
+			// Use left half's attr since the right half of a
+			// wide char may have an unreliable attr value.
 			ScreenLines[off] = ' ';
-			ScreenAttrs[off] = saved_screenattrs[save_off];
+			ScreenAttrs[off] = saved_screenattrs[base_save_off];
 			if (enc_utf8)
 			    ScreenLinesUC[off] = 0;
 			int popup_attr_val =
@@ -4855,10 +4888,53 @@ draw_opacity_padding_cell(
 			screen_char(off, row, col);
 			return;
 		    }
+		    // The content drawing cleared the left half to a
+		    // space (wide char didn't fit at content edge),
+		    // but the saved data has a wide char.  Restore it
+		    // spanning both the content cell and padding cell.
+		    if (base_save_off >= 0
+			    && saved_screenlinesuc[base_save_off] != 0
+			    && utf_char2cells(
+				saved_screenlinesuc[base_save_off]) == 2
+			    && ScreenLines[base_off] == ' '
+			    && ScreenLinesUC[base_off] == 0)
+		    {
+			int popup_attr_val =
+				    get_win_attr(screen_opacity_popup);
+			int blend =
+				    screen_opacity_popup->w_popup_blend;
+
+			ScreenLines[base_off] =
+				    saved_screenlines[base_save_off];
+			ScreenLinesUC[base_off] =
+				    saved_screenlinesuc[base_save_off];
+			ScreenAttrs[base_off] =
+				    saved_screenattrs[base_save_off];
+			ScreenAttrs[base_off] = hl_blend_attr(
+				    ScreenAttrs[base_off],
+				    popup_attr_val, blend, TRUE);
+
+			ScreenLines[off] = 0;
+			ScreenLinesUC[off] = 0;
+			ScreenAttrs[off] = ScreenAttrs[base_off];
+
+			popup_set_base_screen_cell(row, base_col,
+				    ScreenLines[base_off],
+				    ScreenAttrs[base_off],
+				    ScreenLinesUC[base_off]);
+			popup_set_base_screen_cell(row, col,
+				    ScreenLines[off],
+				    ScreenAttrs[off],
+				    ScreenLinesUC[off]);
+			screen_char(base_off, row, base_col);
+			return;
+		    }
 
 		    // Draw padding in the right half.
+		    // Use left half's attr since the right half of a
+		    // wide char may have an unreliable attr value.
 		    ScreenLines[off] = ' ';
-		    ScreenAttrs[off] = saved_screenattrs[save_off];
+		    ScreenAttrs[off] = saved_screenattrs[base_save_off];
 		    if (enc_utf8 && ScreenLinesUC != NULL)
 			ScreenLinesUC[off] = 0;
 		    int popup_attr_val = get_win_attr(screen_opacity_popup);

src/proto/popupwin.pro

@@ -53,6 +53,7 @@ int popup_do_filter(int c);
 int popup_no_mapping(void);
 void popup_check_cursor_pos(void);
 int popup_is_under_opacity(int row, int col);
+int popup_is_over_opacity(int row, int col);
 int popup_is_under_opacity_range(int row, int start_col, int end_col);
 void may_update_popup_mask(int type);
 void may_update_popup_position(void);

src/screen.c

@@ -945,27 +945,36 @@ skip_opacity:
 
 		popup_get_base_screen_cell(row, col + coloff,
 						NULL, &underlying_attr, NULL);
-		ScreenAttrs[off_to] = hl_blend_attr(underlying_attr,
-						combined, blend, FALSE);
 
-		// For double-wide characters, the second cell may have a
-		// different underlying attr (e.g. at popup boundary),
-		// so blend it independently.
+		// For double-wide characters, a terminal cannot render
+		// different background colors for the left and right
+		// halves.  When one half is over a lower opacity popup
+		// and the other is not, use the non-popup side's
+		// underlying attr for both to avoid color leaking.
 		if (char_cells == 2)
 		{
 		    int underlying_attr2 = 0;
+		    int scol1 = col + coloff;
+		    int over1 = popup_is_over_opacity(row, scol1);
+		    int over2 = popup_is_over_opacity(row, scol1 + 1);
 
-		    popup_get_base_screen_cell(row, col + coloff + 1,
+		    popup_get_base_screen_cell(row, scol1 + 1,
 						NULL, &underlying_attr2, NULL);
+		    if (over1 != over2)
+		    {
+			// One half is over a lower popup, the other is
+			// not.  Use the non-popup side for both.
+			if (over1)
+			    underlying_attr = underlying_attr2;
+			else
+			    underlying_attr2 = underlying_attr;
+		    }
 		    ScreenAttrs[off_to + 1] = hl_blend_attr(
 					underlying_attr2, combined, blend,
 					FALSE);
-		    if (blend == 100)
-			resolve_wide_char_opacity_attrs(row,
-				col + coloff, col + coloff + 1,
-				&ScreenAttrs[off_to],
-				&ScreenAttrs[off_to + 1]);
 		}
+		ScreenAttrs[off_to] = hl_blend_attr(underlying_attr,
+						combined, blend, FALSE);
 	    }
 	    else
 #endif
@@ -2330,11 +2339,28 @@ screen_char(unsigned off, int row, int col)
     // output the final blended result.
     // Also suppress if this is a wide character whose second cell
     // is under an opacity popup.
-    if (popup_is_under_opacity(row, col)
-	    || (enc_utf8 && ScreenLinesUC[off] != 0
+    if (popup_is_under_opacity(row, col))
+    {
+	// If this is a wide character whose left half is under an opacity
+	// popup but right half is not, clear the right half so the old
+	// blended value doesn't remain as a ghost after popup_move().
+	if (enc_utf8 && ScreenLinesUC[off] != 0
 		&& utf_char2cells(ScreenLinesUC[off]) == 2
 		&& col + 1 < screen_Columns
-		&& popup_is_under_opacity(row, col + 1)))
+		&& !popup_is_under_opacity(row, col + 1))
+	{
+	    int off2 = off + 1;
+	    ScreenLines[off2] = ' ';
+	    ScreenLinesUC[off2] = 0;
+	    screen_char(off2, row, col + 1);
+	}
+	screen_cur_col = 9999;
+	return;
+    }
+    if (enc_utf8 && ScreenLinesUC[off] != 0
+	    && utf_char2cells(ScreenLinesUC[off]) == 2
+	    && col + 1 < screen_Columns
+	    && popup_is_under_opacity(row, col + 1))
     {
 	screen_cur_col = 9999;
 	return;

src/testdir/dumps/Test_popupwin_opacity_wide_1.dump

@@ -1,13 +1,13 @@
->い*0&#ffffff0|え|ー@15|い|!+&| |1| @3
-|い*&|え|ー@15|い|!+&| |2| @3
-|い*&|え|ー@15|い|!+&| |3| @3
-|い*&|え*0#ffffff16#e000002|ー@6| +&| +0#0000000#ffffff0|ー*&@7|い|!+&| |4| @3
-|い*&| +0#ffffff16#e000002|カ*&|ラ|フ|ル|な| +&|ー*&@1| +&| +0#0000000#ffffff0|ー*&@7|い|!+&| |5| @3
-|い*&| +0#ffffff16#e000002|ポ*&|ッ|プ|ア|ッ|プ|で|─+&|╮| +0#0000000#ffffff0|ー*&@7|い|!+&| |6| @3
-|い*&| +0#ffffff16#e000002|最*&|上|川| +&|ぼ*&|赤|い|な|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |7| @3
-|い*&| +0#ffffff16#e000002|│|あ*&|い|う|え|お|ー@1|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |8| @3
-|い*&| +&|│+0#ffffff16#0000e05|ー*&@6|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |9| @3
-|い*&| +&|╰+0#ffffff16#0000e05|─@13|╯| +0#0000000#ffffff0|ー*&@7|い|!+&| |1|0| @2
+>い*0&#ffffff0|え*0#e08080255#600000255|ー@6| +&| +0#0000000#ffffff0|ー*&@7|い|!+&| |1| @3
+|い*&| +0#e08080255#600000255|カ*0#ffffff255&|ラ|フ|ル|な| +0#e08080255&|ー*&@1| +&| +0#0000000#ffffff0|ー*&@7|い|!+&| |2| @3
+|い*&| +0#e08080255#600000255|ポ*0#ffffff255#600030255|ッ|プ|ア|ッ|プ|で|─+0#e08080255&|╮| +0#0000000#ffffff0|ー*&@7|い|!+&| |3| @3
+|い*&| +0#e08080255#600000255|最*0#ffffff255#600030255|上|川| +0#e08080255&|ぼ*&|赤|い|な|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |4| @3
+|い*&| +0#e08080255#600000255|│+0&#600030255|あ*&|い|う|え|お|ー*0#a04070255&@1|│+0#e08080255&| +0#0000000#ffffff0|ー*&@7|い|!+&| |5| @3
+|い*&| +&|│+0#ffffff255#000060255|ー*0#8080e0255&@6|│+0#ffffff255&| +0#0000000#ffffff0|ー*&@7|い|!+&| |6| @3
+|い*&| +&|╰+0#ffffff255#000060255|─@13|╯| +0#0000000#ffffff0|ー*&@7|い|!+&| |7| @3
+|い*&|え|ー@15|い|!+&| |8| @3
+|い*&|え|ー@15|い|!+&| |9| @3
+|い*&|え|ー@15|い|!+&| |1|0| @2
 |い*&|え|ー@15|い|!+&| |1@1| @2
 |い*&|え|ー@15|い|!+&| |1|2| @2
 |い*&|え|ー@15|い|!+&| |1|3| @2

src/testdir/dumps/Test_popupwin_opacity_wide_2.dump

@@ -1,15 +1,15 @@
 >い*0&#ffffff0|え|ー@15|い|!+&| |1| @3
 |い*&|え|ー@15|い|!+&| |2| @3
-|い*&|え|ー@15|い|!+&| |3| @3
-|い*&|え|ー@15|い|!+&| |4| @3
-|い*&|え|ー@15|い|!+&| |5| @3
-|い*&| +&|╭+0#ffffff16#0000e05|─@13|╮| +0#0000000#ffffff0|ー*&@7|い|!+&| |6| @3
-|い*&| +&|│+0#ffffff16#0000e05|あ*&|め|ん|ぼ|赤|い|な|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |7| @3
-|い*&| +&|│+0#ffffff16#0000e05|あ*&|い|う|え|お| +&| +0&#e000002|ー*&|│+&| |ー*&@5|ー*0#0000000#ffffff0@1|い|!+&| |8| @3
-|い*&| +&|│+0#ffffff16#0000e05|ー*&@4| +&| +0&#e000002|カ*&|ラ|フ|ル|な|ー@1| +&@1|ー*0#0000000#ffffff0@1|い|!+&| |9| @3
-|い*&| +&|╰+0#ffffff16#0000e05|─@10|─+0&#e000002|ポ*&|ッ|プ|ア|ッ|プ|で| +&@1|ー*0#0000000#ffffff0@1|い|!+&| |1|0| @2
-|い*&|え|ー@4| +&| +0#ffffff16#e000002|最*&|上|川|ー@3| +&@1|ー*0#0000000#ffffff0@1|い|!+&| |1@1| @2
-|い*&|え|ー@4| +&| +0#ffffff16#e000002|ー*&@7|ー*0#0000000#ffffff0@1|い|!+&| |1|2| @2
+|い*&| +&|╭+0#ffffff255#000060255|─@13|╮| +0#0000000#ffffff0|ー*&@7|い|!+&| |3| @3
+|い*&| +&|│+0#ffffff255#000060255|あ*&|め|ん|ぼ|赤|い|な|│+&| +0#0000000#ffffff0|ー*&@7|い|!+&| |4| @3
+|い*&| +&|│+0#ffffff255#000060255|あ*&|い|う|え|お|ー*0#8080e0255&@1|│+0#ffffff255&| +0#0000000#ffffff0|ー*&@7|い|!+&| |5| @3
+|い*&| +&|│+0#ffffff255#000060255|ー*0#8080e0255&@4| +&| +0#a04070255#600030255|ー*&|│+0#e08080255&| +0&#600000255|ー*&@5|ー*0#0000000#ffffff0@1|い|!+&| |6| @3
+|い*&| +&|╰+0#ffffff255#000060255|─@10|─+0#e08080255#600030255|カ*0#ffffff255&|ラ*0&#600000255|フ|ル|な|ー*0#e08080255&@2|ー*0#0000000#ffffff0@1|い|!+&| |7| @3
+|い*&|え|ー@4| +&| +0#e08080255#600000255|ポ*0#ffffff255&|ッ|プ|ア|ッ|プ|で|ー*0#e08080255&|ー*0#0000000#ffffff0@1|い|!+&| |8| @3
+|い*&|え|ー@4| +&| +0#e08080255#600000255|最*0#ffffff255&|上|川|ー*0#e08080255&@4|ー*0#0000000#ffffff0@1|い|!+&| |9| @3
+|い*&|え|ー@4| +&| +0#e08080255#600000255|ー*&@7|ー*0#0000000#ffffff0@1|い|!+&| |1|0| @2
+|い*&|え|ー@15|い|!+&| |1@1| @2
+|い*&|え|ー@15|い|!+&| |1|2| @2
 |い*&|え|ー@15|い|!+&| |1|3| @2
 |い*&|え|ー@15|い|!+&| |1|4| @2
 |:| @25|1|,|1| @10|T|o|p| 

src/testdir/test_popupwin.vim

@@ -4932,6 +4932,7 @@ func Test_popup_opacity_wide_char_overlap()
   " higher-zindex popup are properly blended (no holes or missing chars).
   let lines =<< trim END
     set encoding=utf-8
+    set termguicolors
     for i in range(1, 20)
       call setline(i, 'いえーーーーーーーーーーーーーーーーい! ' .. i)
     endfor
@@ -4939,7 +4940,7 @@ func Test_popup_opacity_wide_char_overlap()
     hi MyPopup2 ctermbg=darkred ctermfg=white
     let g:p1 = popup_create(['あめんぼ赤いな','あいうえお'], #{
         \ opacity: 50,
-        \ line: 6,
+        \ line: 3,
         \ col: 4,
         \ border: [],
         \ borderchars: ['─','│','─','│','╭','╮','╯','╰'],
@@ -4950,7 +4951,7 @@ func Test_popup_opacity_wide_char_overlap()
         \})
     let g:p2 = popup_create(['カラフルな','ポップアップで','最上川'], #{
         \ opacity: 50,
-        \ line: 4,
+        \ line: 1,
         \ col: 3,
         \ minwidth: 15,
         \ minheight: 3,
@@ -4963,9 +4964,9 @@ func Test_popup_opacity_wide_char_overlap()
   let buf = RunVimInTerminal('-S XtestPopupOpacityWide', #{rows: 15, cols: 45})
   call VerifyScreenDump(buf, 'Test_popupwin_opacity_wide_1', {})
 
-  " Move popups far apart so they don't overlap.
-  " Tests right edge of popup where wide chars span content/padding boundary.
-  call term_sendkeys(buf, ":call popup_move(g:p2, #{line: 14, col: 16})\<CR>")
+  " Move p2 so it partially overlaps with p1 at a different position.
+  " Tests wide chars at the overlap boundary of two opacity popups.
+  call term_sendkeys(buf, ":call popup_move(g:p2, #{line: 6, col: 16})\<CR>")
   call TermWait(buf)
   call term_sendkeys(buf, ":\<CR>")
   call VerifyScreenDump(buf, 'Test_popupwin_opacity_wide_2', {})

src/version.c

@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    332,
 /**/
     331,
 /**/