Add support for more OSC color formats

[skyline75489]

• Correct the behaviour of parsing rgb:R/G/B. It should be interpreted
  as RR/GG/BB instead of 0R/0G/0B
• Add support for rgb:RRR/GGG/BBB and rgb:RRRR/GGGG/BBBB. The
  behaviour of 12 bit variants is to repeat the first digit at the end,
  e.g. rgb:123/456/789 becomes rgb:1231/4564/7897.
• Add support for # formats. We are following the rules of
  XParseColor by interpreting #RGB as R000G000B000.
• Add support for XOrg app color names, which are supported by xterm, VTE
  and many other terminal emulators.
• Multi-parameter OSC 4 is now supported.
• The chaining of OSC 10-12 is not yet supported. But the parameter
  validation is relaxed by parsing the parameters as multi-params but
  only use the first one, which means \e]10;rgb:R/G/B; and
  \e]10:rgb:R/G/B;invalid will execute OSC 10 with the first color
  correctly. This fixes some of the issues mentioned in Add support for multi-param OSC sequences #942 but not
  all of them.

Closes #3715

[KalleOlaviNiemitalo]

The behaviour of 12 bit variants is to repeat the first digit at the end, e.g. rgb:123/456/789 becomes rgb:1234/4567/7897.

Don't you mean rgb:1231/4564/7897?

[skyline75489]

@KalleOlaviNiemitalo Yes you are correct. Thanks for pointing out.

[skyline75489]

I'd very much love to add the color names support in rgb.txt. But I think that file itself is for sure GPL. If we use sources like wikipedia, will it still count as using GPL-licensed content?

CC @DHowett

[Lo0oG]

I'd very much love to add the color names support in rgb.txt. But I think that file itself is for sure GPL. If we use sources like wikipedia, will it still count as using GPL-licensed content?

You can get a rgb.txt file from XOrg and that's not GPL licensed. https://www.x.org/releases/X11R6.7.0/src/X11R6.7.0-src2.tar.gz
Haven't checked how similar it is to the XParseColor version.
Also see https://en.wikipedia.org/wiki/X11_color_names
Another source could be the HTML specs. https://drafts.csswg.org/css-color/#named-color

[skyline75489]

Thanks @Lo0oG for the information. I’m surprised (also glad) to know that’s not GPL licensed. We are going through the internal legal process for us to be able to use it.

[WSLUser]

What I don't get is GPL is still open-source. So why is it an issue to have GPL and MIT in an MS repo? Is it that the legal department doesn't want to have to deal with both? If so, then they should be overridden. Their job isn't to stop usage of OSS based on a particular license.

[AnuthaDev]

@WSLUser GPL is for "Free" Software, what that means is if you distribute software binaries licensed under GPL, you have to distribute the source code too, MIT on the other hand has no such requirement and MIT licensed software doesn't have to be open source. The problem is that GPL is a "Viral" license, meaning if you include GPL licensed code in your product, you would have to distribute the whole product under GPL, and since some parts of this repo are also synced with the windows source code, it would mean microsoft would have to open-source windows itself, which would be a legal nightmare considering how much code was licensed from third parties(Some of which don't even exist anymore!!). So, while GPL could be used as the license for windows terminal, it can't be used for windows, and hence no GPL licensed code can be included in this repo.

[AnuthaDev]

If so, then they should be overridden.

I don't think microsoft would be too happy with that😅

[skyline75489]

Thanks @AnuthaDev. I wrote basically the same thing then I saw your post and just give up writing my own.

Just want to point out that not allowing GPL code in this repo is actually MS following the rules of the OSS community. Surely one can copy GPL code and keep their work close sourced. And lawsuits about GPL violations are not always feasible. Imagine in what world would XOrg actually sue MS for just some color metadata. It’s not all about getting away from legal issues. It’s also about MS trying to be a good OSS citizen.

[KalleOlaviNiemitalo]

I think it's perfectly fine to keep settings.json supporting only the #rrggbb format and not these OSC color formats. Wouldn't want to list all the color names in the JSON schema.

[DHowett]

You don't have to worry about the 30kb growth for now. We can book work later to try to optimize it.

That's almost literally just the the color table:

• 6943 bytes for 676 null-terminated strings
• 2704 bytes for 676 RGBA quads
• 10816 bytes for 676 char*, size_t tuples at 16 bytes each (yeah, this part sucks)

That's 20kb alone for the whole table stored fairly inefficiently.

[DHowett]

You can put the suppression inside til::at. That's what it's there for -- we use it when we know that the static analyzer is going to complain about something that we've vetted as being safe.

[DHowett]

If we had a prefix trie we could probably remove the duplicate cost of the ~514 colors that are literally just colors with different numbers at the end... which would save approximately 5174 bytes. It would complicate this PR a lot, so I'd rather do it later.

[skyline75489]

Oh crap. I think I messed up the commits. Well this is indeed a lengthy thread. In fact I think this is most commented PR I've ever made on Github. I'll force push later.

[DHowett]

Well, cttrie is out of the question.

Even with half the colors removed:

cl

C:\Users\Dustin\Projects\thirdparty\Terminal\trie\cttrie\getindex.h(26): fatal error C1202: recursive type or function dependency context too complex

clang

./cttrie/getindex.h:27:32: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack
      overflow are likely [-Wstack-exhausted]

(it's still trying to compile, but I had to set the template depth to 1048576.)

[skyline75489]

@DHowett I also tried Trie-Gen and with the 3800 loc it generated, the binary size went up to 1080k. So... you can rule that out, too.

[DHowett]

cl took 7 minutes before emitting an error message, and clang is still going (miraculously)

[skyline75489]

Man those poor compilers. They didn't even know what hit them.

Trie-Gen gave me pure C code which is no burden for compiles but not optimized for binary size.

[j4james]

If you're desperate, you could probably save around 5KB by storing the data as two simple arrays. 2704 bytes for the 676 RGBA quads, and 12351 bytes for the strings (6943 bytes of content and 676 pointers). You'd have to then build the map at startup, but I wouldn't think that would be a huge amount of code. Not sure it's worth the complication though.

[DHowett]

Yeah, I'd say it's not worth it.

I got the table down to ~5kb using a gperf hash and stripping the color names entirely, but that is riddled with false positives because there's no actual check that the key matched.

(the good)

input: peachpuff
lookup: peachpuff
hash: 2059
found! Color #FFFFDAB9

input: AnTiQuE WhItE
lookup: antiquewhite
hash: 432
found! Color #FFFAEBD7

(the bad)

input: not a color
lookup: notacolor
hash: 599
found! Color #FFEEE0E5

input: ffuphcaep
lookup: ffuphcaep
hash: 2059
found! Color #FFFFDAB9

input: peachpuff
lookup: peachpuff
hash: 2059
found! Color #FFFFDAB9

[skyline75489]

So I was thinking what if we store the colorTable with prefix trie in some kind of string representation and compressed it using gzip or zlib, then decompress and reconstruct the trie...

OK let's do gperf hash then. I think the false positives should have very limited real-life impact, at least to "normal" people who don't mess around with their terminal.

[DHowett]

Thanks for your contributions, everyone, and thanks @skyline75489 for working hard on this 😄

[ghost]

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download