Skip to content

Commit 540cf06

Browse files
johnnovakjohnnovak
committed
Clean up parsing of the cga_colors setting
1 parent 1954343 commit 540cf06

File tree

2 files changed

+43
-46
lines changed

2 files changed

+43
-46
lines changed

‎src/ints/int10_modes.cpp

Lines changed: 33 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "pci_bus.h"
3232
#include "render.h"
3333
#include "rgb666.h"
34+
#include "rgb888.h"
3435
#include "setup.h"
3536
#include "string_utils.h"
3637
#include "vga.h"
@@ -2082,7 +2083,7 @@ std::vector<std::string> tokenize_cga_colors_pref(const std::string& cga_colors_
20822083
// Input should be a token output by `tokenize_cga_colors_pref`, representing
20832084
// a color definition. Tokens are assumed to have no leading or trailing
20842085
// white-spaces
2085-
std::optional<Rgb666> parse_color_token(const std::string& token,
2086+
std::optional<Rgb888> parse_color_token(const std::string& token,
20862087
const uint8_t color_index)
20872088
{
20882089
if (token.size() == 0) {
@@ -2119,21 +2120,18 @@ std::optional<Rgb666> parse_color_token(const std::string& token,
21192120
}
21202121

21212122
if (is_hex3_token) {
2122-
auto r = static_cast<uint8_t>(value >> 8 & 0xf);
2123-
auto g = static_cast<uint8_t>(value >> 4 & 0xf);
2124-
auto b = static_cast<uint8_t>(value & 0xf);
2123+
auto r4 = static_cast<uint8_t>(value >> 8 & 0xf);
2124+
auto g4 = static_cast<uint8_t>(value >> 4 & 0xf);
2125+
auto b4 = static_cast<uint8_t>(value & 0xf);
21252126

2126-
r = r | r << 4;
2127-
g = g | g << 4;
2128-
b = b | b << 4;
2127+
return Rgb888::FromRgb444(r4, g4, b4);
21292128

2130-
return Rgb666(r, g, b);
2131-
} else {
2132-
auto r = static_cast<uint8_t>(value >> 16 & 0xff);
2133-
auto g = static_cast<uint8_t>(value >> 8 & 0xff);
2134-
auto b = static_cast<uint8_t>(value & 0xff);
2129+
} else { // hex6 token
2130+
const auto r8 = static_cast<uint8_t>(value >> 16 & 0xff);
2131+
const auto g8 = static_cast<uint8_t>(value >> 8 & 0xff);
2132+
const auto b8 = static_cast<uint8_t>(value & 0xff);
21352133

2136-
return Rgb666(r, g, b);
2134+
return Rgb888(r8, g8, b8);
21372135
}
21382136
}
21392137
case '(': {
@@ -2147,38 +2145,40 @@ std::optional<Rgb666> parse_color_token(const std::string& token,
21472145
const auto g_string = parts[1];
21482146
const auto b_string = parts[2].substr(0, parts[2].size() - 1);
21492147

2150-
auto parse_component = [&](const std::string& component) {
2148+
auto parse_component =
2149+
[&](const std::string& component) -> std::optional<uint8_t> {
21512150
constexpr char trim_chars[] = " \t,";
2152-
auto c = component;
2151+
2152+
auto c = component;
21532153
trim(c, trim_chars);
21542154

21552155
if (c.empty() || !is_digits(c)) {
21562156
log_warning("RGB-triplet values must contain only digits");
2157-
return -1;
2157+
return {};
21582158
}
21592159

21602160
int32_t value;
21612161
if (!sscanf(c.c_str(), "%d", &value)) {
21622162
log_warning("could not parse RGB-triplet value");
2163-
return -1;
2163+
return {};
21642164
}
2165-
if (value > 0xff) {
2165+
if (value < 0 || value > 255) {
21662166
log_warning("RGB-triplet values must be between 0 and 255");
2167-
return -1;
2167+
return {};
21682168
}
21692169
return value;
21702170
};
21712171

2172-
const auto r = parse_component(r_string);
2173-
const auto g = parse_component(g_string);
2174-
const auto b = parse_component(b_string);
2172+
const auto r8 = parse_component(r_string);
2173+
const auto g8 = parse_component(g_string);
2174+
const auto b8 = parse_component(b_string);
21752175

2176-
if (r < 0 || g < 0 || b < 0) {
2177-
return {};
2176+
if (r8 && g8 && b8) {
2177+
return Rgb888(static_cast<uint8_t>(*r8),
2178+
static_cast<uint8_t>(*g8),
2179+
static_cast<uint8_t>(*b8));
21782180
} else {
2179-
return Rgb666(static_cast<uint8_t>(r),
2180-
static_cast<uint8_t>(g),
2181-
static_cast<uint8_t>(b));
2181+
return {};
21822182
}
21832183
}
21842184
default:
@@ -2215,15 +2215,12 @@ std::optional<cga_colors_t> parse_cga_colors(const std::string& cga_colors_setti
22152215
!color) {
22162216
found_errors = true;
22172217
} else {
2218-
// For now we only support 18-bit colours (6-bit
2219-
// components) when redefining CGA colors. There's not
2220-
// too much to be gained by adding full 24-bit support,
2221-
// and it would complicate the implementation a lot.
2222-
color->red >>= 2;
2223-
color->green >>= 2;
2224-
color->blue >>= 2;
2225-
2226-
cga_colors[i] = *color;
2218+
// We only support 18-bit colours (RGB666) when
2219+
// redefining CGA colors. There's not too much to be
2220+
// gained from adding full 24-bit support, and it would
2221+
// complicate the implementation a lot.
2222+
2223+
cga_colors[i] = Rgb666::FromRgb888(*color);
22272224
}
22282225
}
22292226

‎tests/int10_modes_tests.cpp

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
#include <vector>
2929

3030
// declarations of private functions to test
31-
std::optional<Rgb666> parse_color_token(const std::string& token,
31+
std::optional<Rgb888> parse_color_token(const std::string& token,
3232
const uint8_t color_index);
3333

3434
std::optional<cga_colors_t> parse_cga_colors(const std::string& cga_colors_prefs);
@@ -41,23 +41,23 @@ constexpr auto dummy_color_index = 0;
4141

4242
TEST(parse_color_token, hex3_valid)
4343
{
44-
const Rgb666 expected = Rgb666(0x11, 0xaa, 0xee);
44+
const Rgb888 expected = Rgb888(0x11, 0xaa, 0xee);
4545
const auto result = parse_color_token("#1ae", dummy_color_index);
4646
EXPECT_TRUE(result.has_value());
4747
EXPECT_EQ(expected, result);
4848
}
4949

5050
TEST(parse_color_token, hex3_valid_min)
5151
{
52-
const Rgb666 expected = Rgb666(0x00, 0x00, 0x00);
52+
const Rgb888 expected = Rgb888(0x00, 0x00, 0x00);
5353
const auto result = parse_color_token("#000", dummy_color_index);
5454
EXPECT_TRUE(result.has_value());
5555
EXPECT_EQ(expected, result);
5656
}
5757

5858
TEST(parse_color_token, hex3_valid_max)
5959
{
60-
const Rgb666 expected = Rgb666(0xff, 0xff, 0xff);
60+
const Rgb888 expected = Rgb888(0xff, 0xff, 0xff);
6161
const auto result = parse_color_token("#fff", dummy_color_index);
6262
EXPECT_TRUE(result.has_value());
6363
EXPECT_EQ(expected, result);
@@ -101,23 +101,23 @@ TEST(parse_color_token, hex3_invalid_character)
101101

102102
TEST(parse_color_token, hex6_valid)
103103
{
104-
const Rgb666 expected = Rgb666(0x12, 0xab, 0xef);
104+
const Rgb888 expected = Rgb888(0x12, 0xab, 0xef);
105105
const auto result = parse_color_token("#12abef", dummy_color_index);
106106
EXPECT_TRUE(result.has_value());
107107
EXPECT_EQ(expected, result);
108108
}
109109

110110
TEST(parse_color_token, hex6_valid_min)
111111
{
112-
const Rgb666 expected = Rgb666(0x00, 0x00, 0x00);
112+
const Rgb888 expected = Rgb888(0x00, 0x00, 0x00);
113113
const auto result = parse_color_token("#000000", dummy_color_index);
114114
EXPECT_TRUE(result.has_value());
115115
EXPECT_EQ(expected, result);
116116
}
117117

118118
TEST(parse_color_token, hex6_valid_max)
119119
{
120-
const Rgb666 expected = Rgb666(0xff, 0xff, 0xff);
120+
const Rgb888 expected = Rgb888(0xff, 0xff, 0xff);
121121
const auto result = parse_color_token("#ffffff", dummy_color_index);
122122
EXPECT_TRUE(result.has_value());
123123
EXPECT_EQ(expected, result);
@@ -147,23 +147,23 @@ TEST(parse_color_token, hex6_invalid_too_long)
147147

148148
TEST(parse_color_token, rgb_triplet_valid_no_whitespaces)
149149
{
150-
const Rgb666 expected = Rgb666(7, 42, 231);
150+
const Rgb888 expected = Rgb888(7, 42, 231);
151151
const auto result = parse_color_token("(7,42,231)", dummy_color_index);
152152
EXPECT_TRUE(result.has_value());
153153
EXPECT_EQ(expected, result);
154154
}
155155

156156
TEST(parse_color_token, rgb_triplet_valid_single_whitespaces)
157157
{
158-
const Rgb666 expected = Rgb666(7, 42, 231);
158+
const Rgb888 expected = Rgb888(7, 42, 231);
159159
const auto result = parse_color_token("(7, 42, 231)", dummy_color_index);
160160
EXPECT_TRUE(result.has_value());
161161
EXPECT_EQ(expected, result);
162162
}
163163

164164
TEST(parse_color_token, rgb_triplet_valid_multiple_whitespaces)
165165
{
166-
const Rgb666 expected = Rgb666(7, 42, 231);
166+
const Rgb888 expected = Rgb888(7, 42, 231);
167167
const auto result = parse_color_token("( 7 , 42 , 231 )",
168168
dummy_color_index);
169169
EXPECT_TRUE(result.has_value());

0 commit comments

Comments
 (0)