Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setcolor.com has no effect [when output=ttf] #4503

Closed
2 tasks done
Arthur3032 opened this issue Oct 2, 2023 · 11 comments · Fixed by #4505
Closed
2 tasks done

setcolor.com has no effect [when output=ttf] #4503

Arthur3032 opened this issue Oct 2, 2023 · 11 comments · Fixed by #4505
Labels

Comments

@Arthur3032
Copy link

Describe the bug

Z:\BIN\SETCOLOR 2 #005500 --> #00aa00 not #005500

Steps to reproduce the behaviour

image

Z:\BIN\SETCONT
Z:\BIN\SETCOLOR 2 #005500

Expected behavior

No response

What operating system(s) this bug have occurred on?

Windows 11 22H2

What version(s) of DOSBox-X have this bug?

dosbox-x 2023-09-01

Used configuration

No response

Output log

No response

Additional information

No response

Have you checked that no similar bug report(s) exist?

  • I have searched and didn't find any similar bug report.

Code of Conduct & Contributing Guidelines

  • I agree to follow the code of conduct and the contributing guidelines.
@Arthur3032 Arthur3032 added the bug label Oct 2, 2023
@maxpat78
Copy link
Contributor

maxpat78 commented Oct 3, 2023

@Arthur3032 Just tried to SETCOLOR 0 or SETCOLOR 1 ... color change is effective, at least in VS2022 x64 Build - only it reports as command output the previous color, not the target (new) color.

@Arthur3032
Copy link
Author

Please try SETCOLOR 2, SETCOLOR 3

@maxpat78
Copy link
Contributor

maxpat78 commented Oct 4, 2023

I've just tried setcolor 2 #FF0000 with the latest commit (built by me), also: it works as expected, and it changes in real time the color of the "HAVE FUN WITH DOSBox-X !" string displayed by the shell.

Perhaps only a particular build/SDL combo is affected?

@maxpat78
Copy link
Contributor

maxpat78 commented Oct 4, 2023

Update:
I've looked at your screenshot... you're right: SETCOLOR does not work, but only when Video/Output menu is set to "True Type font". Direct3D (default for me), OpenGL, etc. work right.
I suggest you to edit your topic title and change into SETCOLOR has no effect with "TrueType font" video output

@maxpat78
Copy link
Contributor

maxpat78 commented Oct 4, 2023

Note while debugging: setVGADAC in output_ttf.cpp seems getting the right altBGR1 matrix to IO_WriteB, but such change has no effect on display.

@joncampbell123 joncampbell123 changed the title setcolor.com has no effect setcolor.com has no effect [when output=ttf] Oct 4, 2023
@Arthur3032
Copy link
Author

Arthur3032 commented Oct 4, 2023

However, it worked normally before
This will only be the case after v2023-
image

@maron2000
Copy link
Contributor

I haven't understood the code completely, but the following patch seems to work.
There may be some flaws but you can try a build from my repository.
https://github.com/maron2000/dosbox-x-1/actions/runs/6403360447 (VS x64 build)

diff --git a/src/dos/dos_programs.cpp b/src/dos/dos_programs.cpp
index 6e2d7da55..3389a12fa 100644
--- a/src/dos/dos_programs.cpp
+++ b/src/dos/dos_programs.cpp
@@ -8145,7 +8145,9 @@ static void COLOR_ProgramStart(Program * * make) {
     *make=new COLOR;
 }

-bool setVGAColor(const char *colorArray, int i) {
+alt_rgb altBGR[16], *rgbcolors = (alt_rgb*)render.pal.rgb;
+
+bool setVGAColor(const char *colorArray, int j) {
     if (!IS_VGA_ARCH||!CurMode) return false;
     const char * nextRGB = colorArray;
     int rgbVal[4] = {-1,-1,-1,-1};
@@ -8164,16 +8166,18 @@ bool setVGAColor(const char *colorArray, int i) {
     } else
         return false;
     IO_ReadB(mem_readw(BIOS_VIDEO_PORT)+6);
-    IO_WriteB(VGAREG_ACTL_ADDRESS, i+32);
+    IO_WriteB(VGAREG_ACTL_ADDRESS, j+32);
     uint8_t imap=IO_ReadB(VGAREG_ACTL_READ_DATA);
     IO_WriteB(VGAREG_DAC_WRITE_ADDRESS, imap);
-    IO_WriteB(VGAREG_DAC_DATA, (rgbVal[0]+3)*63/255);
-    IO_WriteB(VGAREG_DAC_DATA, (rgbVal[1]+3)*63/255);
-    IO_WriteB(VGAREG_DAC_DATA, (rgbVal[2]+3)*63/255);
+    IO_WriteB(VGAREG_DAC_DATA, rgbVal[0] >> 2);
+    IO_WriteB(VGAREG_DAC_DATA, rgbVal[1] >> 2);
+    IO_WriteB(VGAREG_DAC_DATA, rgbVal[2] >> 2);
+    rgbcolors[j].red = rgbVal[0];
+    rgbcolors[j].green = rgbVal[1];
+    rgbcolors[j].blue = rgbVal[2];
     return true;
 }

-alt_rgb altBGR[16], *rgbcolors = (alt_rgb*)render.pal.rgb;
 #if defined(USE_TTF)
 extern alt_rgb altBGR1[16];
 extern bool colorChanged;
@@ -8275,9 +8279,12 @@ void SETCOLOR::Run()
 #if defined(USE_TTF)
                        } else if (setColors(value,i)) {
                 bool colornul = staycolors || (IS_VGA_ARCH && (altBGR1[i].red > 4 || altBGR1[i].green > 4 || altBGR1[i].blue > 4) && rgbcolors[i].red < 5 && rgbcolors[i].green < 5 && rgbcolors[i].blue < 5);
-                altBGR[i].red = colornul||(colorChanged&&!IS_VGA_ARCH)?altBGR1[i].red:rgbcolors[i].red;
-                altBGR[i].green = colornul||(colorChanged&&!IS_VGA_ARCH)?altBGR1[i].green:rgbcolors[i].green;
-                altBGR[i].blue = colornul||(colorChanged&&!IS_VGA_ARCH)?altBGR1[i].blue:rgbcolors[i].blue;
+                LOG_MSG("%d,%d,%d %d,%d,%d",altBGR[i].red, altBGR[i].green, altBGR[i].blue, rgbcolors[i].red,rgbcolors[i].green,rgbcolors[i].blue);
+                LOG_MSG("colornul=%d, colorChanged=%d, IS_VGA_ARCH=%d",colornul, colorChanged, IS_VGA_ARCH);
+                altBGR[i].red = (colornul||(colorChanged&&!IS_VGA_ARCH))?altBGR1[i].red:rgbcolors[i].red;
+                altBGR[i].green = (colornul||(colorChanged&&!IS_VGA_ARCH))?altBGR1[i].green:rgbcolors[i].green;
+                altBGR[i].blue = (colornul||(colorChanged&&!IS_VGA_ARCH))?altBGR1[i].blue:rgbcolors[i].blue;
+                LOG_MSG("%d,%d,%d %d,%d,%d",altBGR[i].red, altBGR[i].green, altBGR[i].blue, rgbcolors[i].red,rgbcolors[i].green,rgbcolors[i].blue);
                                WriteOut("Color %d => (%d,%d,%d) or #%02x%02x%02x\n",i,altBGR[i].red,altBGR[i].green,altBGR[i].blue,altBGR[i].red,altBGR[i].green,altBGR[i].blue);
                                resetFontSize();
                        } else
diff --git a/src/output/output_ttf.cpp b/src/output/output_ttf.cpp
index 2195df046..3e818d54d 100644
--- a/src/output/output_ttf.cpp
+++ b/src/output/output_ttf.cpp
@@ -242,9 +242,9 @@ void setVGADAC() {
             IO_WriteB(VGAREG_ACTL_ADDRESS, i+32);
             imap[i]=IO_ReadB(VGAREG_ACTL_READ_DATA);
             IO_WriteB(VGAREG_DAC_WRITE_ADDRESS, imap[i]);
-            IO_WriteB(VGAREG_DAC_DATA, (altBGR1[i].red+3)*63/255);
-            IO_WriteB(VGAREG_DAC_DATA, (altBGR1[i].green+3)*63/255);
-            IO_WriteB(VGAREG_DAC_DATA, (altBGR1[i].blue+3)*63/255);
+            IO_WriteB(VGAREG_DAC_DATA, altBGR1[i].red>>2);
+            IO_WriteB(VGAREG_DAC_DATA, altBGR1[i].green>>2);
+            IO_WriteB(VGAREG_DAC_DATA, altBGR1[i].blue>>2);
         }
     }
 }
@@ -261,7 +261,7 @@ bool setColors(const char *colorArray, int n) {
     staycolors = strlen(colorArray) && *colorArray == '+';
     const char* nextRGB = colorArray + (staycolors?1:0);
        uint8_t * altPtr = (uint8_t *)altBGR1;
-       int rgbVal[3] = {-1,-1,-1};
+       int rgbVal[4] = {-1,-1,-1,-1};
        for (int colNo = 0; colNo < (n>-1?1:16); colNo++) {
                if (n>-1) altPtr+=4*n;
                if (sscanf(nextRGB, " ( %d , %d , %d)", &rgbVal[0], &rgbVal[1], &rgbVal[2]) == 3) {     // Decimal: (red,green,blue)
@@ -273,22 +273,24 @@ bool setColors(const char *colorArray, int n) {
                        while (*nextRGB != ')')
                                nextRGB++;
                        nextRGB++;
-               } else if (sscanf(nextRGB, " #%6x", ((unsigned int*)(&rgbVal[0]))) == 1) {
                        // Hexadecimal
-                       if (rgbVal[0] < 0)
+               } else if (sscanf(nextRGB, " #%6x", ((uint32_t*)(&rgbVal[3]))) == 1) {
                // Hexadecimal
+                       if (rgbVal[3] < 0)
                                return false;
                        for (int i = 0; i < 3; i++) {
-                               altPtr[2-i] = rgbVal[0]&255;
-                               rgbVal[0] >>= 8;
+                               rgbVal[2-i] = rgbVal[3]&255;
+                               rgbVal[3] >>= 8;
                        }
                        nextRGB = strchr(nextRGB, '#') + 7;
                } else
                        return false;
                altPtr += 4;
-       }
-       for (int i = n>-1?n:0; i < (n>-1?n+1:16); i++) {
-               altBGR0[i].blue = (altBGR1[i].blue*2 + 128)/4;
-               altBGR0[i].green = (altBGR1[i].green*2 + 128)/4;
-               altBGR0[i].red = (altBGR1[i].red*2 + 128)/4;
+               altBGR1[n].blue = rgbVal[2];
+               altBGR1[n].green = rgbVal[1];
+               altBGR1[n].red = rgbVal[0];
+               rgbColors[n].blue = rgbVal[2];
+               rgbColors[n].green = rgbVal[1];
+               rgbColors[n].red = rgbVal[0];
+
        }
     setVGADAC();
     colorChanged=justChanged=true;

setcolor_test
setcolor_test2

@Arthur3032
Copy link
Author

Thank you, Let me try it.

@maxpat78
Copy link
Contributor

maxpat78 commented Oct 5, 2023

Patch works as expected!

@maron2000
Copy link
Contributor

@maxpat78 Thank you for testing. I'll prepare a PR.

@Arthur3032
Copy link
Author

It's OK, Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants