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

Support 24bit colors #69

Closed
certik opened this issue Feb 12, 2021 · 8 comments · Fixed by #93
Closed

Support 24bit colors #69

certik opened this issue Feb 12, 2021 · 8 comments · Fixed by #93
Assignees
Labels
enhancement New feature or request

Comments

@certik
Copy link
Collaborator

certik commented Feb 12, 2021

Currently cpp-terminal only supports 4bit colors. We should extend it to also support 24 bit colors:

https://en.wikipedia.org/wiki/ANSI_escape_code#24-bit

We have to add a new function color_24bit (similar to the current function color which is only 4 bit) and use the ESC[ 38;2;⟨r⟩;⟨g⟩;⟨b⟩ m sequence to set it.

Then we need to extend the Window class, currently the color is only represented by the 3bit color vector<fg>. Instead we have to change it to vector<Color>, where the Color struct is something like this:

struct Color {
    enum { bit24, bit3, bit4 } tag;
    union {
        struct {
            char R, G, B;
        };   
        fg color_fg;
        fgB color_fgB;
    };  
};   

and then the set_fg method would become:

void set_fg(size_t x, size_t y, fg c) {
    m_fg[(y-1)*w+(x-1)].tag = bit3;    
    m_fg[(y-1)*w+(x-1)].color_fg = c;  
}

void set_fg_24bit(size_t x, size_t y, char R, char G, char B) {
    m_fg[(y-1)*w+(x-1)].tag = bit24;
    m_fg[(y-1)*w+(x-1)].R = R;      
    m_fg[(y-1)*w+(x-1)].G = G;
    m_fg[(y-1)*w+(x-1)].B = B;
}

Also the render method would need to be updated.

@MCWertGaming
Copy link
Collaborator

I have experimented a bit with RGB coloring in cpp-terminal and it's going well so far. I have created the function color24(int, int, int, bool). I know it looks ugly, but how are we going to handle the input of it? The problem with using a struct like color() does isn't possible, as we have 255x255x255 colors. Another problem is that we need to set foreground and background values, as \033[38;2;<r>;<g>;<b>m is only changing the foreground, the background is set with \033[48;2;<r>;<g>;<b>m (just 48 instead of 38).

I would so it like this:

std::string color24(unsigned int red, unsigned int green, unsigned int blue, bool fg)
{
    if (fg)
        return "\033[38;2;" + std::to_string(red) + ';' + std::to_string(green) + ';' + std::to_string(blue) + 'm';
    else
        return "\033[48;2;" + std::to_string(red) + ';' + std::to_string(green) + ';' + std::to_string(blue) + 'm';
}

I have already tested it:
image

What do you think about that?

I have also checked, if color(fg::reset) resets also 24bit colors and it does! But I would suggest to create a function with \033[0m as it's the ANSI reset code and resets all attributes (and is 1 character shorter than \033[39m).

I'll look into the color struct of the window class next.

@MCWertGaming MCWertGaming added the enhancement New feature or request label Feb 23, 2021
@certik
Copy link
Collaborator Author

certik commented Feb 23, 2021

We can have a function for \033[0m in addition to the other functions.

Great job a the 24 bit colors. Something like what you did should work!

@MCWertGaming
Copy link
Collaborator

Yeah! I'll do the changes inside of the window class next. I would also take that opportunity to create a "test" example for the terminal tests, it would include all features of cpp-terminal so we can easily determine which terminals are supporting which things / colors. Also I would like to either add colors24 to the readme or move the color part from the readme into a section of the wiki.

What do you think?
also should color() stay that way, or is color4() better? (because color 4bit)

@certik
Copy link
Collaborator Author

certik commented Feb 23, 2021

That all looks like a great plan. I would leave color for now, we can rename later.

@MCWertGaming MCWertGaming linked a pull request Feb 24, 2021 that will close this issue
@MCWertGaming
Copy link
Collaborator

I have started to edit the parts in the Window class. Your Color struct is not compiling at all due to the -Wall compiler flag, because ISO C++ forbids anonymous structures and unions inside of structures. The correct way to do that is to create the color type enum separately, as well as the rgb struct and create a union to group the 4bit and 24bit colors together. Just have to change a bit because I cant initilize a vector holding a struct like you did with the normal vectors Window() : vector(size, content) I'll probably need some more testing around to get a good approach.

@certik
Copy link
Collaborator Author

certik commented Feb 25, 2021

Sounds good. Let me know if you need help.

@MCWertGaming
Copy link
Collaborator

MCWertGaming commented Feb 28, 2021

A small update: I have tried a few ideas to implement the colors inside of the window class. 24bit color is really complicated, as we can't just re-use all of the code already there. Also the render function gets really blown as we have to check for the tag vector all the time and make big if monsters. I would suggest to simply make it two classes (like Window_4bit and Window_24bit). I think that would make the usage a bit easier as well. But If you would prefer it to be one class, I can try to merge the them together afterwards.

@MCWertGaming
Copy link
Collaborator

Also: the window demo crashes my visual studio code integrated terminal and has some color issues inside of it. That might be a problem on other Terminal as well (just the window example though). We should test that in #73 as well to be safe. It is possible, that the visual studio terminal has some problems, because the same binary just works perfectly fine on my normal terminal (Kitty). I open a separate issue on that one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants