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

Colors: Update Color System (TrueColor, Blink, Underline, etc...) for v2 (Master Issue) #457

Open
8 tasks
tig opened this issue May 20, 2020 · 24 comments
Open
8 tasks
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) enhancement
Milestone

Comments

@tig tig added enhancement design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) labels May 20, 2020
@tig tig changed the title Update Color Systems Update Color System May 20, 2020
@tig tig added this to the v2.0 milestone Aug 4, 2022
@tig tig removed this from the v2.0 milestone Feb 28, 2023
@tig tig changed the title Update Color System Colors: Update Color System for v2 (Master Issue) Apr 16, 2023
@tig tig changed the title Colors: Update Color System for v2 (Master Issue) Colors: Update Color System (TrueColor, Blink, Underline, etc...) for v2 (Master Issue) Apr 16, 2023
@tig
Copy link
Collaborator Author

tig commented May 24, 2023

The fix for #666, PR #2612 is providing groundwork for this. Anyone choosing to start on this PR should base it on that PR if you do it before merged in to v2_develop.

@adstep
Copy link
Contributor

adstep commented Jun 25, 2023

Hey @tig, is #2612 still the right branch to start from if I'm playing around with porting over the changes from #1628?

@tznind
Copy link
Collaborator

tznind commented Jun 25, 2023

Hey @tig, is #2612 still the right branch to start from if I'm playing around with porting over the changes from #1628?

That's correct, it is the most recent console driver refactoring branch. Please can you also consider taking/adapting this veeman#2 (tznind:true-color) PR which I opened into #1628 it adds the following:

  • UICatalog scenario for opening images
  • Reverts the new TrueColorAttribute change to instead make Attribute implicitly support/true at the same time with fall back to basic colors when not supported by driver/environment.

I would really love true color support for so many cool things. Especially for colored icons in FileDialog, gradient borders etc. Let me know if you want to collaborate on this.

@adstep
Copy link
Contributor

adstep commented Jun 26, 2023

Sure I can give it a try this week. Seems straightforward enough given the pre-existing examples.

Is this the right place to loop back if I have any troubles?

@tznind
Copy link
Collaborator

tznind commented Jun 26, 2023

Sure I can give it a try this week. Seems straightforward enough given the pre-existing examples.

Awesome, this is really exciting!

Is this the right place to loop back if I have any troubles?

Sure, post any initial/logistics issues here. When there is a PR (even a draft one) then we can talk about more specifics there.

I guess since #2612 is +10,135/−9,977 vs v2_develop it makes most sense to branch off tig:v2_fixes_666_console_driver_dupes and probably target tigs branch with a draft PR with the updated true color code. Otherwise the diff will be lost in all the other changes (a lot are just layout) and hard to review.

We can always retarget once tig:v2_fixes_666_console_driver_dupes is merged if @tig wants to merge them in seperately.

@tig
Copy link
Collaborator Author

tig commented Jun 26, 2023

It will likely be a week or so before I can spend time on my console dupe PR, so I concur you should party on it and submit PRs to it for the color stuff.

Really excited to see someone picking this up! Thank you!

@adstep
Copy link
Contributor

adstep commented Jun 26, 2023

Got it running. See tig#15.

@BDisp
Copy link
Collaborator

BDisp commented Jun 27, 2023

Got it running. See tig#15.

This is a great work. Thanks for providing this. Does it also works on another drivers (CursesDriver and NetDriver)?

@tznind
Copy link
Collaborator

tznind commented Jun 27, 2023

Its ok if it only works for console driver to start with.

We can always add support in the other drivers after. As long as the fallback mechanism is working and the true color class design is sound.

The original work was done by @veeman so it might be we don't have the expertise between us to do the other drivers yet.

@adstep
Copy link
Contributor

adstep commented Jun 27, 2023

Does it also works on another drivers (CursesDriver and NetDriver)?

The PR doesn't add support for CursesDriver or NetDriver.

The PR just enables WindowsDriver to use VT sequences to describe the requested console color. This should work consistently because there's a clear mechanism to detect whether TrueColor support is possible by checking the version of Windows.

AFAIK Curses uses a mechanism where, if you aren't using one of the 16 default Console colors, you have to register the color you want to use and get returned an identifier to pass along for future calls. This has some limitation where you can only have a couple thousand cached colors due to some restrictions of Curses. If you want to use VT sequences, when using Curses, you'd need to figure out some other means of detecting whether they'd be supported.

@tznind
Copy link
Collaborator

tznind commented Jun 27, 2023

If you want to use VT sequences, when using Curses, you'd need to figure out some other means of detecting whether they'd be supported.

I think this should be handled with config files in the same way we support Nerd Fonts (#2613).

So any end user could add to their .tui folder config:

"VTTrueColor.Enable": true

Or if the vast majority of modern consoles support this we could default it to true and let end users opt out with a setting of false in config.

@BDisp
Copy link
Collaborator

BDisp commented Jun 27, 2023

@adstep I don't know if you already seen my explanation why some colors has the value -1, in tig#15 (comment).

@BDisp
Copy link
Collaborator

BDisp commented Jul 6, 2023

Great stuff here:
https://gist.github.com/fnky/458719343aabd01cfb17a3a4f7296797?permalink_comment_id=4619910#gistcomment-4619910

@tznind
Copy link
Collaborator

tznind commented Jul 8, 2023

@BDisp is it still possible to output specific escape sequences in CursesDriver?

I know @tig mentioned reducing/eliminating reliance on ncurses methods in #2612 (comment) in favour of direct use of escape codes (he references #1107)

I found and tested some of the console escape codes in the following video on my linux box and found they work. Also sounds like most modern linux terminals are going to support at least 256 if not true color escapes:
https://www.youtube.com/watch?v=RKPsd4tD9dQ

See this test script that shows how to output escape sequences for 256 colors
https://github.com/BrodieRobertson/scripts/blob/ae0077d38777151e07824410fade622eb93f0ca0/color256#L4

If the escape codes approach taken in tig#15 would still work in linux then I think we should just be practical about things. Specifically:

  • Assume that 256 colors are supported
  • Provide a way to switch to 16 or true color (via ConfigurationManager)
  • Ideally centralize the logic in base ConsoleDriver
  • Ignore the fact that user can remap 256 / 16 color to custom colors (if a user uses custom color schemes we can assume they are happy for that to be applied universally).

Sorry that I am a little late to the party on this as I haven't gotten so deep into all the various schemes etc

@BDisp
Copy link
Collaborator

BDisp commented Jul 8, 2023

@tznind I really don't know, because the CursesDriver call Refresh to write to the screen. In NetDriver we control the writing to the screen and we send escape sequences to a StringBuilder and work well. In CursesDriver maybe is possible to write escape sequences strings to the buffer and perhaps the Refresh can send well them to the screen. But I'm not certain.

@tig
Copy link
Collaborator Author

tig commented Jul 9, 2023

Based on my research I believe @tznind is right.

I am going to resist trying to do this in my huge pr, so I can get it merged asap, but I'm now convinced we can actually replace BOTH CursesDriver and WindowsDriver with a new "AnsiDriver".

@tig
Copy link
Collaborator Author

tig commented Jul 9, 2023

Oh, and if we do end up keeping support for 16 bit color, it will NOT be the default.

Based on what I know now, I want True Color to be default with "as smart as possible" fallback to 256 and/or 16.

I worry about performance emitting stuff ourselves. It's easy to make it work, but my intuition is it's also easy to emit way too many sequences potentially causing perf problems.

@tig
Copy link
Collaborator Author

tig commented Jul 10, 2023

#2612 now passes all unit tests - The true color support is fragile though.

I'm going to refactor a bunch of it in line with what I said above in next few days. Hopefully I can get it to a point where it's architecturally correct, if not completely working, in the next few days. I then have a few more issues in #2612 to address before it can be merged.

@tig
Copy link
Collaborator Author

tig commented Jul 10, 2023

Proposal:

  • Change Color to be a class. Merge TrueColor into it, so that there is a single class that represents a color. Default behavior will be 24-bit/TrueColor. Build in "16 color/256 color fallback"; including letting devs do Color.Red etc... for well known color names.
  • Get rid of TrueColor as a separate class, and simplify Attribute to just hold two Color members as it did previously
  • 'Remove the concept of Initialized and HasValidColors from Attribute given a 24-bit color is always valid.
  • Start with WindowsDriver (which already mostly works). Then get CursesDriver working. Then NetDriver.
  • Keep all three existing drivers for now. Make building a stronger "ANSI" back-end another Issue.

Once the above is done, I can merge to v2_develop.

We can then open PRs to fix/adapt other things:

  • Update ColorPicker
  • Combine/Simpifly the color related scenarios

@BDisp, @tznind, @adstep - let me know if you have more thoughts, questions, or suggestions on all this!

@BDisp
Copy link
Collaborator

BDisp commented Jul 10, 2023

Did the PR #2729 resolved true color on Linux?

@tig
Copy link
Collaborator Author

tig commented Jul 10, 2023

Did the PR #2729 resolved true color on Linux?

I don't know. I misunderstood what was going on, and didn't realize it was different set of work than what @adstep did. So I just closed #2729 without looking at it.

Not sure how to handle this now that you've pointed it out.

@BDisp
Copy link
Collaborator

BDisp commented Jul 10, 2023

Well the PR #2729 includes true color for Linux. I think you must reconsider it.

@tig
Copy link
Collaborator Author

tig commented Jul 10, 2023

Are you sure it does? I peeked at the code briefly and saw no evidence of related changes to CursesDriver.

I'll look again later.

@tig
Copy link
Collaborator Author

tig commented Sep 25, 2024

Found this:

https://github.com/WilliamRagstad/ANSIConsole

Might be useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issues regarding Terminal.Gui design (bugs, guidelines, debates, etc...) enhancement
Projects
Status: 📋 Approved - Need Owner
Status: 🏗 Approved - In progress
Development

No branches or pull requests

4 participants