[PATCH] curses: correctly pass color and attributes to setcchar()
The current code uses getcchar() and setcchar() to handle the cchar_t
values, which is correct, however it incorrectly deconstructs the chtype
value that it then passes to setcchar():
1. The bit mask 0xff against the chtype is not guaranteed to be
correct. curses provides the 'A_ATTRIBUTES' and 'A_CHARTEXT' masks
to do this.
2. The color pair has to be passed to setcchar() separately, any color
pair provided in the attributes is ignored.
The first point is not that big of a deal, the 0xff mask is very likely
to be correct. The second issue however results in colors no longer
working when using the curses display, instead the text will always be
white on black.
This patch fixes the color issue by using PAIR_NUMBER() to retrieve the
color pair number from the chtype value, and then passes that number to
setcchar. Along with that, the hardcoded 0xff masks are replaced with
A_ATTRIBUTES and A_CHARTEXT.
Signed-off-by: Matthew Kilgore
---
ui/curses.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ui/curses.c b/ui/curses.c
index ec281125acbd..3a1b71451c93 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -75,14 +75,16 @@ static void curses_update(DisplayChangeListener *dcl,
line = screen + y * width;
for (h += y; y < h; y ++, line += width) {
for (x = 0; x < width; x++) {
-chtype ch = line[x] & 0xff;
-chtype at = line[x] & ~0xff;
+chtype ch = line[x] & A_CHARTEXT;
+chtype at = line[x] & A_ATTRIBUTES;
+short color_pair = PAIR_NUMBER(line[x]);
+
ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
if (ret == ERR || wch[0] == 0) {
wch[0] = ch;
wch[1] = 0;
}
-setcchar(&curses_line[x], wch, at, 0, NULL);
+setcchar(&curses_line[x], wch, at, color_pair, NULL);
}
mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
}
--
2.23.0
[PATCH v2 1/2] curses: use the bit mask constants provided by curses
The curses API provides the A_ATTRIBUTES and A_CHARTEXT bit masks for
getting the attributes and character parts of a chtype, respectively. We
should use provided constants instead of using 0xff.
Signed-off-by: Matthew Kilgore
---
ui/curses.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/curses.c b/ui/curses.c
index ec281125acbd..84003f56a323 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -75,8 +75,8 @@ static void curses_update(DisplayChangeListener *dcl,
line = screen + y * width;
for (h += y; y < h; y ++, line += width) {
for (x = 0; x < width; x++) {
-chtype ch = line[x] & 0xff;
-chtype at = line[x] & ~0xff;
+chtype ch = line[x] & A_CHARTEXT;
+chtype at = line[x] & A_ATTRIBUTES;
ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
if (ret == ERR || wch[0] == 0) {
wch[0] = ch;
--
2.23.0
[PATCH v2 0/2] curses: fix attribute passing
This patch set fixes up ui/curses.c. A previous change to ui/curses.c,
commit 962cf8fd4fae ("ui/curses: manipulate cchar_t with standard curses
functions"), did not correctly pass the attributes from the chtype to
`setcchar()`.
The biggest issue this caused is that colors no longer work when using
the curses display, it instead renders everything in white on black.
This was fixed by correctly passing the color pair number to setcchar().
I also fixed two spots where 0xff was used instead of the bit mask
constants that are part of the curses API.
changes in v2:
- Split into two patches, one dealing with the attribute masks, and one
dealing with correctly passing the color pair number.
Thanks,
Matthew Kilgore
Matthew Kilgore (2):
curses: use the bit mask constants provided by curses
curses: correctly pass the color pair to setcchar()
ui/curses.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
--
2.23.0
[PATCH v2 2/2] curses: correctly pass the color pair to setcchar()
The current code does not correctly pass the color pair information to
setcchar(), it instead always passes zero. This results in the curses
output always being in white on black.
This patch fixes this by using PAIR_NUMBER() to retrieve the color pair
number from the chtype value, and then passes that value as an argument
to setcchar().
Signed-off-by: Matthew Kilgore
---
ui/curses.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ui/curses.c b/ui/curses.c
index 84003f56a323..3a1b71451c93 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -77,12 +77,14 @@ static void curses_update(DisplayChangeListener *dcl,
for (x = 0; x < width; x++) {
chtype ch = line[x] & A_CHARTEXT;
chtype at = line[x] & A_ATTRIBUTES;
+short color_pair = PAIR_NUMBER(line[x]);
+
ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
if (ret == ERR || wch[0] == 0) {
wch[0] = ch;
wch[1] = 0;
}
-setcchar(&curses_line[x], wch, at, 0, NULL);
+setcchar(&curses_line[x], wch, at, color_pair, NULL);
}
mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
}
--
2.23.0
Re: [PATCH] curses: correctly pass color and attributes to setcchar()
Hi Philippe,
On Thu, Oct 03, 2019 at 09:36:56AM +0200, Philippe Mathieu-Daudé wrote:
Hi Matthew,
On 10/3/19 2:18 AM, Matthew Kilgore wrote:
The current code uses getcchar() and setcchar() to handle the cchar_t
values, which is correct, however it incorrectly deconstructs the chtype
value that it then passes to setcchar():
1. The bit mask 0xff against the chtype is not guaranteed to be
correct. curses provides the 'A_ATTRIBUTES' and 'A_CHARTEXT' masks
to do this.
2. The color pair has to be passed to setcchar() separately, any color
pair provided in the attributes is ignored.
You clearly describe 2 different changes in the same patch.
Do you mind splitting in 2 atomic patches?
Done, I've sent a new patch set with these changes split into two
patches.
Thanks,
Matthew Kilgore
The first point is not that big of a deal, the 0xff mask is very likely
to be correct. The second issue however results in colors no longer
working when using the curses display, instead the text will always be
white on black.
This patch fixes the color issue by using PAIR_NUMBER() to retrieve the
color pair number from the chtype value, and then passes that number to
setcchar.
Along with that, the hardcoded 0xff masks are replaced with
A_ATTRIBUTES and A_CHARTEXT.
This comment would go in the 1st patch.
Signed-off-by: Matthew Kilgore
---
ui/curses.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/ui/curses.c b/ui/curses.c
index ec281125acbd..3a1b71451c93 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -75,14 +75,16 @@ static void curses_update(DisplayChangeListener *dcl,
line = screen + y * width;
for (h += y; y < h; y ++, line += width) {
for (x = 0; x < width; x++) {
-chtype ch = line[x] & 0xff;
-chtype at = line[x] & ~0xff;
+chtype ch = line[x] & A_CHARTEXT;
+chtype at = line[x] & A_ATTRIBUTES;
+short color_pair = PAIR_NUMBER(line[x]);
+
ret = getcchar(&vga_to_curses[ch], wch, &attrs, &colors, NULL);
if (ret == ERR || wch[0] == 0) {
wch[0] = ch;
wch[1] = 0;
}
-setcchar(&curses_line[x], wch, at, 0, NULL);
+setcchar(&curses_line[x], wch, at, color_pair, NULL);
}
mvwadd_wchnstr(screenpad, y, 0, curses_line, width);
}
