Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Background color handling changed between 1.1 and 1.2 #7603

Closed
thomthom opened this issue Sep 11, 2020 · 5 comments
Closed

Background color handling changed between 1.1 and 1.2 #7603

thomthom opened this issue Sep 11, 2020 · 5 comments
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.

Comments

@thomthom
Copy link

Environment

Windows build number: Microsoft Windows [Version 10.0.18363.900] and Microsoft Windows [Version 10.0.19041.450]
Windows Terminal version (if applicable):

Any other software?
PDCurses: https://github.com/wmcbrine/PDCurses @ 618e0aa

Steps to reproduce

Related to the same PDCurses based application that prompted me to report #7594 I found something else. The background handling in Windows Terminal changed between 1.1 and 1.2.

In my app I declare a custom color:

  #define SW_COLOR_DEBUG 100
  init_color(SW_COLOR_DEBUG, 500, 500, 500);
  init_pair(1, SW_COLOR_DEBUG, COLOR_BLACK);

Expected behavior

What I saw in WT 1.1 was what this:

wt-1 1

I wasn't sure if it would work like that, since I had custom non-black background, but I had hoped the black color was remapped.
COLOR_BLACK is defined as 0 in PDCurses, I didn't find any way to set no color.

Actual behavior

In WT 1.2 I'm now seeing actual black.

wt-1 2

I'm not 100% sure what is technically correct here, if I'd seen WT1.2 first I would have assumed my code was wrong, but since there's behaviour change between 1.1 I figured I'd report this.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 11, 2020
@WSLUser
Copy link
Contributor

WSLUser commented Sep 11, 2020

#6506 changed it so now the correct behavior is defined. So yes you need to update your code.

@DHowett
Copy link
Member

DHowett commented Sep 11, 2020

Yep. There's a similar issue that existed in powershell until we fixed it. It explains a bit about why this happens.

PowerShell/PSReadLine#830 (comment)

@DHowett
Copy link
Member

DHowett commented Sep 11, 2020

(thanks @WSLUser)

@thomthom
Copy link
Author

Thanks for the references. I'll read through that and see if I can apply it to PDCurses. I think nurses let you use -1 as "no color change" but PDCurses doesn't seem to allow that. Maybe I have to propose a change to PDCurses.

@DHowett DHowett added Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 11, 2020
@DHowett DHowett closed this as completed Sep 11, 2020
@thomthom
Copy link
Author

thomthom commented Sep 12, 2020

I've been reading the references, but I still don't fully understand what is going on. I made some additional tests:

#include <cassert>
#include <iomanip>
#include <iostream>

#include <curses.h>

#define ISOK(api_function_call) {\
  [[maybe_unused]] int curses_result = api_function_call;\
  if(curses_result != OK)\
    std::cout << "Curses Error: " << curses_result << "\n";\
  assert(curses_result == OK);\
}

void print_input(int input);

int main()
{
  initscr();
  start_color();
  use_default_colors();
  // assume_default_colors(-1, -1);
  raw();
  keypad(stdscr, TRUE);
  noecho();

  mousemask(BUTTON1_CLICKED, nullptr);

  move(1, 1);
  attron(A_BOLD);
  printw("PDCurses Mouse Test");
  attroff(A_BOLD);

  mvprintw(5, 1, "has_colors: %d", has_colors());
  mvprintw(6, 1, "can_change_color: %d", can_change_color());
  mvprintw(7, 1, "COLORS: %d", COLORS);
  mvprintw(8, 1, "COLOR_PAIRS: %d", COLOR_PAIRS);

  short r, g, b;
  color_content(0, &r, &g, &b);
  mvprintw(10, 1, "COLOR_BLACK: %d, %d, %d", r, g, b);

  const int kColorDefaultBg = 200;
  ISOK(init_color(kColorDefaultBg, r, g, b));

  const int kColorDebug = 210;
  ISOK(init_color(kColorDebug, r + 30, g + 20, b + 10));

  ISOK(init_pair(1, COLOR_RED, COLOR_BLACK));
  ISOK(init_pair(2, COLOR_GREEN, COLOR_BLACK));
  ISOK(init_pair(3, COLOR_BLUE, COLOR_BLACK));
  ISOK(init_pair(4, COLOR_CYAN, COLOR_BLACK));
  ISOK(init_pair(5, COLOR_MAGENTA, COLOR_BLACK));
  ISOK(init_pair(6, COLOR_YELLOW, COLOR_BLACK));

  ISOK(init_pair(10, COLOR_RED, -1));

  const int kColorGrey = 100;
  ISOK(init_color(kColorGrey, 500, 500, 500));
  ISOK(init_pair(11, kColorGrey, COLOR_BLACK));
  ISOK(init_pair(12, kColorGrey, -1));

  ISOK(init_pair(13, COLOR_RED, kColorDefaultBg));
  ISOK(init_pair(14, kColorGrey, kColorDefaultBg));
  ISOK(init_pair(15, COLOR_RED, kColorDebug));

  const int kTop = 12;
  for (int i = 0; i < 16; ++i) {
    auto result = color_set(i, NULL);
    mvprintw(kTop + i, 1, "Color Set: %d", i);

    attron(A_BOLD);
    mvprintw(kTop + i, 20, "Color Set: %d", i);
    attroff(A_BOLD);

    attron(A_DIM);
    mvprintw(kTop + i, 40, "Color Set: %d", i);
    attroff(A_DIM);

    attron(A_STANDOUT);
    mvprintw(kTop + i, 60, "Color Set: %d", i);
    attroff(A_STANDOUT);
  }

  bool run = true;
  while(run) {
    refresh();
    int input = getch();

    switch(input) {
    case 'q':
      run = false;
      break;
    default:
      print_input(input);
    }
  }

  endwin();
  return 0;
}

The result in WT 1.2 is:

image

Here is what I don't fully understand:

When I use only the predefined COLOR_ macros in PDCurses and COLOR_BLACK as background it looks fine. (Color set 1-6)

In color set 10 I set foreground to COLOR_RED and background to -1 which also looks fine. (-1 gets normalized to 0 so it ends up being the same as COLOR_BLACK.

But, with any custom foreground color, the background is 0, 0, 0. Set 11 and 12 both ends up passing color 0 from PDCurses, similar to line 10. But yet they render with different color?

From what I understand there are some color mapping going on, but is the logic different if either foreground or background is different from the default colors (or different from one of the indexed 0-16 colors)? Does a custom foreground color affect how the background color is mapped? (If not, why would set 11 and 12 have different background from 10 in the example above?)

I why is 12-13 using black background when I define a color that is the same as the terminal defines? (Which I can even read back from color 0.)
(Edit: Never mind this color set 12-13, I forgot the colors from PDCurses are 0-1000 and 47 is a very low value. So I don't understand where that value comes from.)

I found this article: https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#extended-colors
But I'm not sure if that relate only to cmd.exe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Tag-Fix Doesn't match tag requirements Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons.
Projects
None yet
Development

No branches or pull requests

3 participants