Use fixed render sizes and SDL_RenderSetLogicalSize for scaling #275

xordspar0 · 2022-10-25T04:11:24Z

Previously we had a range of possible resolutions of different aspect ratios. Each of these resolutions could be windowed or full screen. This change simplifies the renderer a bit. Now the renderer only supports two resolutions: the original 320x240 and a widescreen resolution of 432x243. The widescreen resolution was chosen as the integer resolution that is closest to 240 lines tall that is also has 16:9 aspect ratio.

Now the display settings are also simpler. You can choose 1x, 2x, or 3x scale, or fullscreen. Widescreen is a separate option. SDL automaticaly scales up with nearest neighbor interpolation. In fullscreen mode, if the renderer aspect ratio doesn't match the user's screen ratio, the display is simply letterboxed.

This change comes with the following benefits:

We don't need to keep track of all the possible resolutions a user might want. We only need two: a 4:3 and a 16:9 resolution. Other ratios could be added.
When we use scaling and letterboxing, we don't have to worry about problems like Incorrect camera dimensions at non-4:3 resolutions #245 that cause strange things to be visible.
In my opinion, fullscreen looks better with letterboxing and nearest neighbor scaling than the stretching and linear interpolation that we used before when we simply did SDL_SetWindowDisplayMode.
The user's desktop HighDPI scale settings are automatically respected. Previously, users that had HighDPI scaling enabled had to manually choose a resolution from the list smaller than their monitor's real resolution.
Using SDL_FULLSCREEN_DESKTOP instead of changing the monitor's mode is faster, smoother, and integrates better with the user's desktop. On Wayland, changing the monitor's mode isn't even possible in SDL.
On top of all this, the render code is notably smaller and simpler because there's no need for scale checks sprinkled throughout the code.

There are currently a few problems that mean this isn't ready for merging yet:

~~Maps are off-center in widescreen mode. See "kings" intro for example. This relates to recalc_map_offsets() and map.maxxscroll.~~

Example
Flickering outside render area in fullscreen mode; looks like uninitialized memory. SDL docs say the display should be letterboxed and the letterboxed area should just be black. This is because SDL_RenderClear is never called anywhere. It should be called once per frame.
~~Inventory position doesn't update to center of screen if you switch to/from widescreen while the inventory is open~~
~~At some monitor resolutions in fullscreen, some sprites have overdraw. This is a GPU bug that affects certain GPU models. I implemented a workaround.~~

Example
I don't own a Switch and Vita that I can test with, but I have simulated their screens with appropriately sized windows.

Fixes #171.
Fixes #245.
Fixes #276.

isage · 2022-10-25T07:15:39Z

newScale = 1; - that looks wrong for switch (which used 4) and vita (which used 2).
Also, letterboxing would look bad on vita (which is 16:10)

xordspar0 · 2022-10-25T13:55:32Z

newScale = 1; - that looks wrong for switch (which used 4) and vita (which used 2).

The scale is only used for the window size. It doesn't matter for fullscreen. I guess those newScale = 1; lines could be deleted. They don't do anything.

Also, letterboxing would look bad on vita (which is 16:10)

For the Via and Switch I use custom resolutions to avoid letterboxing -- the same resolutions that they use today.

xordspar0 · 2022-12-11T21:42:55Z

Status update: I think this is looking really good. It's just about ready, but I want to get more testing done before marking it as ready. ~~I had my friend test it and there's a bug with his Nvidia GPU that I can't reproduce on my Intel or AMD GPUs.~~

Update: my friend's driver has a bug unrelated to NXengine or my changes.

xordspar0 · 2023-02-06T19:05:20Z

I have been playing with this version for the last couple months and it has been working flawlessly for me. It's ready to consider for merging.

Without this, the whole game is rendered at 1x resolution and linearly scaled up on high DPI screens.

If the user switches to/from widescreen mode while the inventory is open, the position of the inventory will now update instead of being offset one way or the other. Also, it's not necessary to adjust the y position of the inventory based on whether we're in widescreen mode because the screen height doesn't change.

Same as the inventory menu.

The SDL docs say this needs to be called before drawing to the render buffer, even if you plan to paint every pixel. Not calling it was causing graphical glitches outside the render area when the render area didn't fill the whole screen, such as when rendering a 4:3 viewport on a 16:9 monitor.

The x position of the camera used to be manually calculated for maps that are less wide than the screen. Now it's calcualted automatically so that these maps are always in the center of the screen, regardless of screen width.

Switch is 16:9, so the standard widescreen resolution should work fine.

To be honest, I thought this is what SDL_RenderSetLogicalSize() did, but actually it scales up each object that you draw with SDL_RenderCopy(). This is necessary because on some GPUs, scaling up can cause a sprite to bleed into an adjacent frame in the spritesheet texture. This shouldn't happen with nearest neighbor scaling, but it nevertheless does happen on some GPUs such as my laptop's embedded Intel GPU. On this GPU I saw a one pixel wide strip on the top and left of every sprite that showed the neighboring sprite frame. The strip was one pixel wide in my screen's pixels, not the logical resolution's pixels. Rendering everything to a 320x240 texture and then scaling up the whole texture at once makes strange bugs like this impossible.

xordspar0 · 2023-04-26T16:48:09Z

Rebased

Renderer::initVideo now depends on Sprites being initialized because it calls setResolution. If the sprites aren't initialized, the Renderer can get a segfault in flushAll() when it tries to check which spritesheets are initialized and which aren't. Sprites can only detect whether the spritesheets are initialized after Sprites::init has been called, which zeroes out the spritesheets. This bug only appeared on my Raspberry Pi, so I didn't notice it in the last several months of testing. Presumably the other systems I tested on had this uninitialized memory set to all zeroes by chance.

bburky · 2024-04-15T17:09:32Z

This patch worked great for me. I was playing on an RGB30 device which has a 1:1 screen. The old scaling actually has a bug (I can open an issue if you want), where it seems like the rendered viewport doesn't match the actual area of the game rendered. I was seeing some blocks flicker in and out on the right side of the screen.

This patch as is correctly draws the correct screen area only and draws black outside the configured aspect ratio.

(I haven't tested the current code, I built before this branch was recently rebased)

bburky · 2024-04-15T17:09:58Z

src/graphics/Renderer.cpp

+    newWidth  = 432;
+    newHeight = 243;


This code was also much easier to hack on and add a custom aspect ratio. I manually patched these "widescreen" values to be a 1:1 full screen aspect ratio. I accidentally the code I used for my build, I'm unsure what exact values I used.

I wonder, would it make sense to make these "widescreen" values configurable? Perhaps from an environment variable or something?

When I worked on this at first, I had in mind that 320x240 was the "one true resolution" since that's what the original game had, but I added widescreen as a concession because nxengine-evo already had support for it.

Since then I've thought a lot about it. I think it could be nice to add support for a variety of ratios. The main constraint is that the vertical resolution should be 240 pixels (or very close to it). We could either have a list of hand-picked resolutions like we have here, or we could even have an algorithm that automatically makes a resolution for your aspect ratio that meets the requirements. But letting the user manually choose a resolution or aspect ratio that's not on the list is also a good idea.

In any case, I don't want to add any new features to this already quite large PR. If it's merged I would be happy to look into ideas like these.

xordspar0 · 2024-04-16T15:51:19Z

This patch worked great for me. I was playing on an RGB30 device which has a 1:1 screen.

Cool!

The old scaling actually has a bug (I can open an issue if you want), where it seems like the rendered viewport doesn't match the actual area of the game rendered. I was seeing some blocks flicker in and out on the right side of the screen.

I believe I noticed the same thing while working on this PR and fixed it in fdbcf01. That could probably be backported to the main branch if this PR isn't accepted.

xordspar0 mentioned this pull request Oct 25, 2022

Incorrect camera dimensions at non-4:3 resolutions #245

Open

xordspar0 force-pushed the camera-dimensions branch 2 times, most recently from 8ff2c57 to 3ca02b4 Compare November 27, 2022 17:04

xordspar0 mentioned this pull request Nov 28, 2022

Fullscreen doesn't work well on Wayland #276

Open

xordspar0 force-pushed the camera-dimensions branch from c72a478 to 486f979 Compare February 6, 2023 19:01

xordspar0 marked this pull request as ready for review February 6, 2023 19:05

xordspar0 added 16 commits April 26, 2023 11:47

Use SDL_WINDOW_FULLSCREEN_DESKTOP for fullscreen mode

9176d1c

Set the correct render size when switching to fullscreen

b3738aa

Always render at the base size; let SDL scale for us

0c0c05b

Allow high DPI

323e4b2

Without this, the whole game is rendered at 1x resolution and linearly scaled up on high DPI screens.

Don't hardcode dimensions

8cdbd62

Clean up extra ()

775e0e2

Factor out scale field

7b3a5cf

Don't adjust the save select menu y position based on widescreen mode

462e076

Same as the inventory menu.

Factor out duplicate code

b042789

Automatically calculate the camera position for skinny maps

e49d109

The x position of the camera used to be manually calculated for maps that are less wide than the screen. Now it's calcualted automatically so that these maps are always in the center of the screen, regardless of screen width.

Correct centering for maps thinner than the screen

8f90b14

Don't use a custom resolution for Switch

b7bd4e8

Switch is 16:9, so the standard widescreen resolution should work fine.

Factor out duplicate code between initVideo and setResolution

15e475f

xordspar0 force-pushed the camera-dimensions branch from 486f979 to 15e475f Compare April 26, 2023 16:47

xordspar0 added 2 commits September 30, 2023 15:14

Merge branch 'master' into camera-dimensions

5fb1179

Merge branch 'master' into camera-dimensions

56e9b27

bburky reviewed Apr 15, 2024

View reviewed changes

mhoogcarspel mentioned this pull request Apr 18, 2024

Port Issue: Cave Story Evo PortsMaster/PortMaster-New#115

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fixed render sizes and SDL_RenderSetLogicalSize for scaling #275

Use fixed render sizes and SDL_RenderSetLogicalSize for scaling #275

xordspar0 commented Oct 25, 2022 •

edited

Loading

isage commented Oct 25, 2022

xordspar0 commented Oct 25, 2022 •

edited

Loading

xordspar0 commented Dec 11, 2022 •

edited

Loading

xordspar0 commented Feb 6, 2023

xordspar0 commented Apr 26, 2023

bburky commented Apr 15, 2024 •

edited

Loading

bburky Apr 15, 2024 •

edited

Loading

xordspar0 Apr 16, 2024 •

edited

Loading

xordspar0 commented Apr 16, 2024 •

edited

Loading

Use fixed render sizes and SDL_RenderSetLogicalSize for scaling #275

Are you sure you want to change the base?

Use fixed render sizes and SDL_RenderSetLogicalSize for scaling #275

Conversation

xordspar0 commented Oct 25, 2022 • edited Loading

isage commented Oct 25, 2022

xordspar0 commented Oct 25, 2022 • edited Loading

xordspar0 commented Dec 11, 2022 • edited Loading

xordspar0 commented Feb 6, 2023

xordspar0 commented Apr 26, 2023

bburky commented Apr 15, 2024 • edited Loading

bburky Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

xordspar0 Apr 16, 2024 • edited Loading

Choose a reason for hiding this comment

xordspar0 commented Apr 16, 2024 • edited Loading

xordspar0 commented Oct 25, 2022 •

edited

Loading

xordspar0 commented Oct 25, 2022 •

edited

Loading

xordspar0 commented Dec 11, 2022 •

edited

Loading

bburky commented Apr 15, 2024 •

edited

Loading

bburky Apr 15, 2024 •

edited

Loading

xordspar0 Apr 16, 2024 •

edited

Loading

xordspar0 commented Apr 16, 2024 •

edited

Loading