Page MenuHomeFreeBSD

Make vt(4) palette configurable
ClosedPublic

Authored by gonzo on Dec 26 2017, 9:33 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 31 2024, 1:52 AM
Unknown Object (File)
Jan 12 2024, 10:07 AM
Unknown Object (File)
Dec 23 2023, 3:29 AM
Unknown Object (File)
Dec 10 2023, 6:31 PM
Unknown Object (File)
Nov 15 2023, 8:35 PM
Unknown Object (File)
Nov 8 2023, 1:12 PM
Unknown Object (File)
Nov 8 2023, 11:08 AM
Unknown Object (File)
Nov 4 2023, 4:14 AM
Subscribers

Details

Summary

vt(4) uses hardcoded values for standard 16-colors VGA palette
and the only way to modify it at the monent is by patching kernel.
This patch introduces set of loader variables that can be set
to alter pallete values. Variable name pattern is kern.vt.color.N.rgb
where N is a pallete index varying from 0 to 15. The value is
either coma-separated rgb triplet with values in a range of 0 to 255,
i.e. "255,255,255". Or HTML-like hex-triple, i.e: "#ffffff".

These values can be configured in loader.conf(5)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 13912
Build 14114: arc lint + arc unit

Event Timeline

This revision is now accepted and ready to land.Dec 27 2017, 8:47 AM

It should be documented in loader.conf(5), otherwise looks okay.

Document new tunable in vt(4)

This revision now requires review to proceed.Dec 29 2017, 1:08 AM
In D13645#285283, @manu wrote:

It should be documented in loader.conf(5), otherwise looks okay.

I checked loader.conf it doesn't have much of vt specific info, looks like all tunables are in vt(4). So I put description there

Typo fix.

share/man/man4/vt.4
214

s/coma/comma/

jilles requested changes to this revision.Dec 29 2017, 2:28 PM
jilles added a subscriber: jilles.

Looks useful.

sys/dev/vt/colors/vt_termcolors.c
102

Hmm, perhaps supporting only #rrggbb is sufficient; this reduces verbose and error-prone code. If so, ignore my comments about the r,g,b parser below :)

103

Given that strtoul() depends on the '\0', passing along the length may not be very useful.

112

Given that '\0' is not a space and not a comma, the condition on the length is redundant.

Also, it seems unusual to redundantly parenthesize arithmetic within a relational operator or relational operators within logical operators.

139

Note that any characters after the blue value are ignored. Is this deliberate?

156–159

Perhaps change color_def from 0..100 to 0..255, since it is an implementation detail of vt_termcolors.c. This makes the code slightly easier to understand and improves accuracy.

This revision now requires changes to proceed.Dec 29 2017, 2:28 PM
gonzo added inline comments.
sys/dev/vt/colors/vt_termcolors.c
102

I think having comma-separated triplets is useful. Normally people want to migrate their color scheme from whatever format they have and it's either hex triplets or rgb triplets (in my experience). Supporting both formats may save some users' time.

139

Nope, it's oversight from my side. I added this check and some fixes to the parser. I didn't want to make "r,g.b" parser too strict and enforce comma-only format, hence the whitespaces check. But there is no reason to ignore garbage at the end of the string

156–159

It makes sense to have color_def in percentages because it may be applied to different bits-per-pixel values. On the other hand most common format from user perspective is 8-bit color component specification. IMO it's better to convert from user visible data to internal representation than make palette application logic less readable.

  • Fixed typo in man page
  • Addressed jilles@ comments in rgb-triplet parsing code
sys/dev/vt/colors/vt_termcolors.c
156–159

Provided there is no overflow, the maximum of 100 or 255 simply appears in #define CF below. This is just a bit of churn of the color_def table.

sys/dev/vt/colors/vt_termcolors.c
156–159

I think switching from 100 to 255 is a marginal improvement and not strictly required for the configurable palette feature. What IMO would be better from accuracy point of view is switching to 0..255 range in a palette and using number of bits per color component and logical shift instead of Nmax and dividing by non-power of two numbers. I believe this should be done as a separate patch

sys/dev/vt/colors/vt_termcolors.c
156–159

Just realized "division to shift" part of the statement above is not entirely correct, it will have to be shift + some rounding logic.

jilles added inline comments.
sys/dev/vt/colors/vt_termcolors.c
103–104

This while loop could be omitted, since strtoul() already skips leading whitespace.

156–159

I agree that this need not be fixed for this commit.

This revision is now accepted and ready to land.Dec 30 2017, 10:46 PM

Remove unneccessary loop, strtoul takes care of leading whitespaces

This revision now requires review to proceed.Dec 31 2017, 12:00 AM
This revision is now accepted and ready to land.Dec 31 2017, 11:26 AM
This revision was automatically updated to reflect the committed changes.