Terminal.Gui icon indicating copy to clipboard operation
Terminal.Gui copied to clipboard

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

Open tig opened this issue 5 years ago • 24 comments

This is the master Issue for revamping the Color system in v2. Dependencies:

  • #2381
  • #2797
  • #2798
  • #2799
  • #219
  • #2801
  • #2800

See also:

  • #415

Todos

  • [ ] Add these:
    • [ ] Bright/Bold
    • [ ] Dim
    • [ ] Underline
    • [ ] Blink
    • [ ] Reverse
    • [ ] Hidden,
    • [ ] Strike-through

tig avatar May 20 '20 20:05 tig

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.

tig avatar May 24 '23 07:05 tig

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

adstep avatar Jun 25 '23 08:06 adstep

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 https://github.com/veeman/gui.cs/pull/2 (tznind:true-color) PR which I opened into https://github.com/gui-cs/Terminal.Gui/pull/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.

tznind avatar Jun 25 '23 08:06 tznind

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?

adstep avatar Jun 26 '23 06:06 adstep

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.

tznind avatar Jun 26 '23 07:06 tznind

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!

tig avatar Jun 26 '23 14:06 tig

Got it running. See https://github.com/tig/Terminal.Gui/pull/15.

adstep avatar Jun 26 '23 23:06 adstep

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)?

BDisp avatar Jun 27 '23 13:06 BDisp

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.

tznind avatar Jun 27 '23 17:06 tznind

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.

adstep avatar Jun 27 '23 17:06 adstep

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.

tznind avatar Jun 27 '23 18:06 tznind

@adstep I don't know if you already seen my explanation why some colors has the value -1, in https://github.com/tig/Terminal.Gui/pull/15#discussion_r1243574508.

BDisp avatar Jun 27 '23 18:06 BDisp

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

BDisp avatar Jul 06 '23 16:07 BDisp

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

I know @tig mentioned reducing/eliminating reliance on ncurses methods in https://github.com/gui-cs/Terminal.Gui/pull/2612#issuecomment-1565264597 in favour of direct use of escape codes (he references https://github.com/gui-cs/Terminal.Gui/discussions/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 https://github.com/tig/Terminal.Gui/pull/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

tznind avatar Jul 08 '23 19:07 tznind

@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.

BDisp avatar Jul 08 '23 23:07 BDisp

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 avatar Jul 09 '23 15:07 tig

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 avatar Jul 09 '23 15:07 tig

#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 avatar Jul 10 '23 16:07 tig

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!

tig avatar Jul 10 '23 17:07 tig

Did the PR #2729 resolved true color on Linux?

BDisp avatar Jul 10 '23 17:07 BDisp

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.

tig avatar Jul 10 '23 20:07 tig

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

BDisp avatar Jul 10 '23 21:07 BDisp

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 avatar Jul 10 '23 22:07 tig