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

Pixel Perfect screen scaling #147

Closed
Clovergruff opened this Issue Nov 17, 2017 · 18 comments

Comments

Projects
None yet
5 participants
@Clovergruff

Clovergruff commented Nov 17, 2017

Hi!

I'd like to request a feature/option for rendering the game in such a way that would provide consistent pixel sizes on the screen. This may not be obvious or visible to a lot of people, but that's how the game is supposed to look like.

If you look at some dithered checkerboard-like areas, you can see how the pixels are actually different sizes:
nonpixelperfect

But they should all be the same size:
pixelperfect

Here's a direct comparison:
versus

There's a modified version of Dosbox that supports this feature. I assume it can be used for a reference in case if this feature would be considered!
Link here: https://github.com/bladeSk/DOSBox-pixel-perfect

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Nov 19, 2017

Owner

It seems that SDL has a function for exactly this: https://wiki.libsdl.org/SDL_RenderSetIntegerScale

Owner

NagyD commented Nov 19, 2017

It seems that SDL has a function for exactly this: https://wiki.libsdl.org/SDL_RenderSetIntegerScale

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Nov 19, 2017

Owner

Here is my implementation on a new branch: https://github.com/NagyD/SDLPoP/commits/integer_scaling
You need to enable this in SDLPoP.ini, the option is called use_integer_scaling.
Please try it.

TODO: What to do if 4:3 aspect ratio is also enabled? Currently the game window stays black.
(I added a warning to the INI, and you also get a warning on the console if you enable both settings.)

This happens due to how the 4:3 aspect ratio is implemented: The screen is first scaled to a very big size that has the correct aspect ratio and its coordinates are integer multiples of 320×200 (namely: 320 * 5 × 200 * 6 = 1600 × 1200), and then that is scaled down to the current size of the window.
This is bigger than the biggest possible size of the window (i.e. full screen), so the only suitable integer scaling factor is zero.

The same thing happens if you don't enable 4:3 aspect ratio, but resize the window to a size smaller than 320×200.

Owner

NagyD commented Nov 19, 2017

Here is my implementation on a new branch: https://github.com/NagyD/SDLPoP/commits/integer_scaling
You need to enable this in SDLPoP.ini, the option is called use_integer_scaling.
Please try it.

TODO: What to do if 4:3 aspect ratio is also enabled? Currently the game window stays black.
(I added a warning to the INI, and you also get a warning on the console if you enable both settings.)

This happens due to how the 4:3 aspect ratio is implemented: The screen is first scaled to a very big size that has the correct aspect ratio and its coordinates are integer multiples of 320×200 (namely: 320 * 5 × 200 * 6 = 1600 × 1200), and then that is scaled down to the current size of the window.
This is bigger than the biggest possible size of the window (i.e. full screen), so the only suitable integer scaling factor is zero.

The same thing happens if you don't enable 4:3 aspect ratio, but resize the window to a size smaller than 320×200.

@Falcury

This comment has been minimized.

Show comment
Hide comment
@Falcury

Falcury Nov 19, 2017

Contributor

I just tested it. It works well.
I think the integer scaling should probably be suppressed while the window size is smaller than the logical render size.
I suppose that people with 1440p monitors will be able to use both features together.

Contributor

Falcury commented Nov 19, 2017

I just tested it. It works well.
I think the integer scaling should probably be suppressed while the window size is smaller than the logical render size.
I suppose that people with 1440p monitors will be able to use both features together.

NagyD added a commit that referenced this issue Nov 25, 2017

Disable integer scaling while the window size is smaller than the log…
…ical render size.

Otherwise the picture would disappear.
See issue #147.
@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Nov 25, 2017

Owner

I think the integer scaling should probably be suppressed while the window size is smaller than the logical render size.

Done.

I suppose that people with 1440p monitors will be able to use both features together.

Probably. I added a note to SDLPoP.ini, saying that at least 1600 × 1200 is needed in that case.
(On my system, the maximum resolution is 1440 × 900.)

Owner

NagyD commented Nov 25, 2017

I think the integer scaling should probably be suppressed while the window size is smaller than the logical render size.

Done.

I suppose that people with 1440p monitors will be able to use both features together.

Probably. I added a note to SDLPoP.ini, saying that at least 1600 × 1200 is needed in that case.
(On my system, the maximum resolution is 1440 × 900.)

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Dec 2, 2017

Owner

So, can I close the issue and merge the branch?

Owner

NagyD commented Dec 2, 2017

So, can I close the issue and merge the branch?

@Falcury

This comment has been minimized.

Show comment
Hide comment
@Falcury

Falcury Dec 2, 2017

Contributor

Maybe people will have trouble compiling/running if they are not yet on SDL 2.0.5, like Norbert mentioned here:
http://forum.princed.org/viewtopic.php?f=126&t=4061#p22226

I suppose a compile time check will be sufficient for letting people compile on older versions.
Something like:

#if SDL_VERSION_ATLEAST(2,0,5)
...
#endif

The only remaining problem would be if people on GNU/Linux having SDL 2.0.4 or lower try to run a pre-built binary. (They would have to recompile using their own installed pre-2.0.5 version of SDL, otherwise the game will probably complain that the procedure entry point SDL_RenderSetIntegerScale could not be located in the dynamic link library.)

Contributor

Falcury commented Dec 2, 2017

Maybe people will have trouble compiling/running if they are not yet on SDL 2.0.5, like Norbert mentioned here:
http://forum.princed.org/viewtopic.php?f=126&t=4061#p22226

I suppose a compile time check will be sufficient for letting people compile on older versions.
Something like:

#if SDL_VERSION_ATLEAST(2,0,5)
...
#endif

The only remaining problem would be if people on GNU/Linux having SDL 2.0.4 or lower try to run a pre-built binary. (They would have to recompile using their own installed pre-2.0.5 version of SDL, otherwise the game will probably complain that the procedure entry point SDL_RenderSetIntegerScale could not be located in the dynamic link library.)

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Dec 15, 2017

Owner

I suppose a compile time check will be sufficient for letting people compile on older versions.

Done: ed86fd4

The only remaining problem would be if people on GNU/Linux having SDL 2.0.4 or lower try to run a pre-built binary.

We could use dlopen+dlsym on GNU/Linux (and LoadLibrary+GetProcAddress on Windows).
But that seems to be overkill...
And anyway, we are not distributing pre-built SDLPoP binaries for GNU/Linux. (Or are we?)

So, can I merge this into master, or are there still problems?

Owner

NagyD commented Dec 15, 2017

I suppose a compile time check will be sufficient for letting people compile on older versions.

Done: ed86fd4

The only remaining problem would be if people on GNU/Linux having SDL 2.0.4 or lower try to run a pre-built binary.

We could use dlopen+dlsym on GNU/Linux (and LoadLibrary+GetProcAddress on Windows).
But that seems to be overkill...
And anyway, we are not distributing pre-built SDLPoP binaries for GNU/Linux. (Or are we?)

So, can I merge this into master, or are there still problems?

@EndeavourAccuracy

This comment has been minimized.

Show comment
Hide comment
@EndeavourAccuracy

EndeavourAccuracy Dec 15, 2017

Contributor

And anyway, we are not distributing pre-built SDLPoP binaries for GNU/Linux. (Or are we?)

All the ZIP packages at popot.org have 64-bit ELF executables.
Compiled and added by me to make the package even end-user-friendlier.
(Of course, everyone is free to compile their own instead.)

Contributor

EndeavourAccuracy commented Dec 15, 2017

And anyway, we are not distributing pre-built SDLPoP binaries for GNU/Linux. (Or are we?)

All the ZIP packages at popot.org have 64-bit ELF executables.
Compiled and added by me to make the package even end-user-friendlier.
(Of course, everyone is free to compile their own instead.)

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Dec 18, 2017

Owner

All the ZIP packages at popot.org have 64-bit ELF executables.
Compiled and added by me to make the package even end-user-friendlier.

Oh, sorry, I didn't know about that.
(But I at least asked "are we?". :) )

However, if you compile those with SDL 2.0.4 as you wrote here, then we don't have to worry about the problem with SDL_RenderSetIntegerScale outlined by Falcury.
Instead, the integer scaling will be simply disabled, with a warning that tells to recompile with SDL 2.0.5 to enable it.

So, can I merge now?

Owner

NagyD commented Dec 18, 2017

All the ZIP packages at popot.org have 64-bit ELF executables.
Compiled and added by me to make the package even end-user-friendlier.

Oh, sorry, I didn't know about that.
(But I at least asked "are we?". :) )

However, if you compile those with SDL 2.0.4 as you wrote here, then we don't have to worry about the problem with SDL_RenderSetIntegerScale outlined by Falcury.
Instead, the integer scaling will be simply disabled, with a warning that tells to recompile with SDL 2.0.5 to enable it.

So, can I merge now?

@Shane32

This comment has been minimized.

Show comment
Hide comment
@Shane32

Shane32 Jan 5, 2018

I use dosbox in a configuration where it doubles the 320x200 to 640x400, then does a "smooth" scaling of the 640x400 output to the native resolution of the monitor (with aspect correction so it maintains the correct ratio). I find this gives both the most pleasant and authentic representation of a 1990-era computer, because the pixels are just a bit fuzzy. Note that I think this is done by DosBox via "hardware scaling" with either opengl or direct3d, as it doesn't work in the other output modes.

Shane32 commented Jan 5, 2018

I use dosbox in a configuration where it doubles the 320x200 to 640x400, then does a "smooth" scaling of the 640x400 output to the native resolution of the monitor (with aspect correction so it maintains the correct ratio). I find this gives both the most pleasant and authentic representation of a 1990-era computer, because the pixels are just a bit fuzzy. Note that I think this is done by DosBox via "hardware scaling" with either opengl or direct3d, as it doesn't work in the other output modes.

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Jan 6, 2018

Owner

@Shane32: Here is my first attempt on a new branch: 9c9d2ad
Please try it.

(BTW, I think this should be a separate issue.)

Owner

NagyD commented Jan 6, 2018

@Shane32: Here is my first attempt on a new branch: 9c9d2ad
Please try it.

(BTW, I think this should be a separate issue.)

@Falcury

This comment has been minimized.

Show comment
Hide comment
@Falcury

Falcury Jan 13, 2018

Contributor

@Shane32: Here is my first attempt on a new branch: 9c9d2ad
Please try it.

I tried it. One issue I found: you can sometimes see vertical 'lines' on the left and right edges of the room. This happens especially in fullscreen mode. See for example level 2, or level 12, where this can be clearly seen.

Contributor

Falcury commented Jan 13, 2018

@Shane32: Here is my first attempt on a new branch: 9c9d2ad
Please try it.

I tried it. One issue I found: you can sometimes see vertical 'lines' on the left and right edges of the room. This happens especially in fullscreen mode. See for example level 2, or level 12, where this can be clearly seen.

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Jan 14, 2018

Owner

Yes, I'm seeing something similar.
It seems that each edge of the screen wraps around to the other side.
If I move the prince to the edge and make him turn around, then the "line" on the other edge will move together with the prince.

(I wanted to attach a screenshot, but GitHub gave me an error.)

The good thing: This is a known SDL bug: https://bugzilla.libsdl.org/show_bug.cgi?id=1878
A patch was made in 2013, and it was finally accepted in 2017 December.

Owner

NagyD commented Jan 14, 2018

Yes, I'm seeing something similar.
It seems that each edge of the screen wraps around to the other side.
If I move the prince to the edge and make him turn around, then the "line" on the other edge will move together with the prince.

(I wanted to attach a screenshot, but GitHub gave me an error.)

The good thing: This is a known SDL bug: https://bugzilla.libsdl.org/show_bug.cgi?id=1878
A patch was made in 2013, and it was finally accepted in 2017 December.

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Jan 21, 2018

Owner

I added an INI option to change the scaling mode: 36635f6
This way you can switch between sharp and fuzzy pixels.

Owner

NagyD commented Jan 21, 2018

I added an INI option to change the scaling mode: 36635f6
This way you can switch between sharp and fuzzy pixels.

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Feb 3, 2018

Owner

@Falcury, do you think I can merge fuzzy_pixels to master?

Owner

NagyD commented Feb 3, 2018

@Falcury, do you think I can merge fuzzy_pixels to master?

@Falcury

This comment has been minimized.

Show comment
Hide comment
@Falcury

Falcury Feb 3, 2018

Contributor

Yes, I think so!
Will you make fuzzy pixels and 4:3 scaling the new default? (Seeing as they are changed in SDLPoP.ini)

A patch was made in 2013, and it was finally accepted in 2017 December.

I tested it with the latest version from here:
https://github.com/SDL-mirror/SDL

And it indeed looks like the issue is fixed. Then, I also think it would be a good idea to package a newer SDL2.dll with the release that doesn't have the bug.

I also integrated the fuzzy pixels feature into the menu branch:
Falcury@28222e6

Contributor

Falcury commented Feb 3, 2018

Yes, I think so!
Will you make fuzzy pixels and 4:3 scaling the new default? (Seeing as they are changed in SDLPoP.ini)

A patch was made in 2013, and it was finally accepted in 2017 December.

I tested it with the latest version from here:
https://github.com/SDL-mirror/SDL

And it indeed looks like the issue is fixed. Then, I also think it would be a good idea to package a newer SDL2.dll with the release that doesn't have the bug.

I also integrated the fuzzy pixels feature into the menu branch:
Falcury@28222e6

@NagyD

This comment has been minimized.

Show comment
Hide comment
@NagyD

NagyD Feb 4, 2018

Owner

Yes, I think so!

Right, I merged it: 4fd76bc

Will you make fuzzy pixels and 4:3 scaling the new default? (Seeing as they are changed in SDLPoP.ini)

No, I did that only to "show off" the feature on the separate branch.
I reverted INI settings to default now: 4fd76bc

Then, I also think it would be a good idea to package a newer SDL2.dll with the release that doesn't have the bug.

Do you mean your custom-compiled SDL2.dll?

Owner

NagyD commented Feb 4, 2018

Yes, I think so!

Right, I merged it: 4fd76bc

Will you make fuzzy pixels and 4:3 scaling the new default? (Seeing as they are changed in SDLPoP.ini)

No, I did that only to "show off" the feature on the separate branch.
I reverted INI settings to default now: 4fd76bc

Then, I also think it would be a good idea to package a newer SDL2.dll with the release that doesn't have the bug.

Do you mean your custom-compiled SDL2.dll?

@NagyD NagyD closed this Feb 4, 2018

@Falcury

This comment has been minimized.

Show comment
Hide comment
@Falcury

Falcury Feb 4, 2018

Contributor

Do you mean your custom-compiled SDL2.dll?

For example, yes.
Although if fuzzy pixels is not the default, then I think it's not as much of a problem (more a minor annoyance for the people that want to use the feature). You could just use SDL 2.0.7, if you feel it's better to stick to the stable SDL2 releases.

Contributor

Falcury commented Feb 4, 2018

Do you mean your custom-compiled SDL2.dll?

For example, yes.
Although if fuzzy pixels is not the default, then I think it's not as much of a problem (more a minor annoyance for the people that want to use the feature). You could just use SDL 2.0.7, if you feel it's better to stick to the stable SDL2 releases.

@NagyD NagyD added the enhancement label Apr 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment