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

Fixed color in some modes under x86-64 MacOS SDL2 #3428

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

myon98
Copy link
Contributor

@myon98 myon98 commented Apr 24, 2022

From issue #1431, originally fixed for SDL1 at #614, there still seem to be some issue with regards to video and MacOS when using SDL2.

I'm not too familiar with the codebase, but from a quick glance through it it seems that DosBox-X assumes the pixel format returned from SDL_GetWindowSurface() without checking, which seems to be ARGB on everywhere else while BGRA on MacOS SDL1.

It seems that in some places where the conversions for MacOS occur there is a check to specifically target SDL1 OSX, while others apply it to SDL2 as well. I've added some at render_templates.h which fixed the issue for my particular situation.

There are other places that looks like this however, such as in sdlmain.cpp, vga_draw.cpp, output_opengl.cpp, but I didn't include them in this commit, as I'm not too sure about my understanding of the codebase.

I've only tested it on my Win 3.1 guest, as I don't have a convenient way to test many video configurations. Also as this Mac is not mine I don't have a way to test it very frequently. I'd like to ask for comments and contributions from people more experienced with this project or using the MacOS SDL2 port frequently.

Both screenshot from MacOS 11 (Big Sur) x86-64. First is the latest binary release MacOS SDL2 0.83.24, next is the latest git master with this PR applied. Guest is Windows 3.10 Japanese, with svga_s3 in 1024x768 64k colors.

dosbox-x-sdl2
dosbox-x-sdl2-fixed

@myon98
Copy link
Contributor Author

myon98 commented Apr 24, 2022

Reading the issue I've linked to, I think I should have tested it with UniVBE. I'll post the results if I can test it tomorrow.

@myon98 myon98 changed the title Fixed color in some modes under MacOS SDL2 Fixed color in some modes under x86-64 MacOS SDL2 Apr 25, 2022
@myon98
Copy link
Contributor Author

myon98 commented Apr 25, 2022

Testing various color modes with vbetest has shown that the first commit only fixed the issue in 15- or 16-bit modes but remained yellow in 32-bit mode. With an additional fix in vga_draw.cpp, 8, 15, 16, and 32-bit modes all display correctly.

I've attached some test result screenshots below (filenames are cocoa-sdl2-${output}-${patch}-${resolution}-${bitdepth})
vbetest.zip

Tested on MacOS 11 (Big Sur) x86-64, unfortunately I don't have access to ARM Macs.

I feel like ideally those ifdefs should be replaced with a common C_PIXFMT_BGRA or something like that to prevent inconsistency like this, but I don't fully understand the codebase and the exact boundaries where the conversions should happen to do that yet.

@brunocastello
Copy link

How can I test by compiling my own ARM build with this PR?

@rderooy
Copy link
Contributor

rderooy commented Apr 28, 2022

You need to clone the git repo and then you can use the github gh pr checkout 3428 command to switch to the pr branch and build it. You do need the github cli installed.

Other option is to clone the repo and download the changed files.

@brunocastello
Copy link

brunocastello commented Apr 28, 2022

You need to clone the git repo and then you can use the github gh pr checkout 3428 command to switch to the pr branch and build it. You do need the github cli installed.

Other option is to clone the repo and download the changed files.

I did a quick search and found that git fetch REMOTE pull/PRNUMBER/head:BRANCHNAME also works without having to install the git cli.

@brunocastello
Copy link

@rderooy @myon98 just built a SDL2 version for M1 Mac, it works. I booted a fresh new Win 98 SE install and I've tested with both 16 and 32 bit colors. No more yellow tinted screen.

@Wengier
Copy link
Collaborator

Wengier commented Apr 29, 2022

I think people like @emendelson had complained several times about such an issue. Very nice fix indeed. Thanks @myon98.

@joncampbell123 joncampbell123 merged commit 635300b into joncampbell123:master Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows 3.11 screen colors with Mac SDL2 build?
5 participants