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

Render resolution restriction and window improvements #1272

Closed
wants to merge 7 commits into from
Closed

Render resolution restriction and window improvements #1272

wants to merge 7 commits into from

Conversation

johnnovak
Copy link
Member

@johnnovak johnnovak commented Sep 18, 2021

Note
As this ticket also serves as documentation temporarily, I've updated the description so the config param names reflect the current reality.


I almost raised three separate PRs for this, but then decided to just do it in one because it's easier to explain why I need these features through a few examples that show how they can be used together. In any case, there are separate commits for the three features.

This is my first PR so please let me know if I missed something :)

Motivation

Case 1

I like to play 320x200 games in fullscreen on my 24" 1920x1080 monitor with the draw area restricted to a 960x720 rectangle at the center of screen. That closely matches the size of a typical 14" VGA display from a normal viewing distance.

space-quest-3

This is how I always play low-resolution games in ScummVM and WinUAE; I find anything larger just too big, the pixels are so huge -- that's not how those games looked on a 14" monitor back in the day!

Right now, I could use windowed mode with the default sharp shader and set the window resolution to 960x720, but I find the window decorations and the desktop very immersion-breaking when playing a game.

Case 2

When I play old grid-based RPGs, I like to have the emulator window on the left side of the screen, and my own dungeon mapper program on the right side. For maximum immersion, I hide the taskbar and disable the window decorations on both windows. Then I can comfortably Alt-Tab between the two windows when playing the game.

eye-of-the-beholder-no-titlebar

All this can be easily done in WinUAE. To replicate it in DOSBox, we need the same draw area restriction mechanism from Case 1. We also need the ability to set the exact window size and window position from the config (without any window size refinement), and an option to hide all window decorations.

Solution

It's perhaps a bit involved to explain the solution in detail, but it's really intuitive and straighforward to use, as evidenced by the config examples further down. I've spend a considerable time to make this 100% backward compatible at default settings and logical, but if you can think of some further improvements, I'm happy to hear them :)

  1. Introduce new viewport_resolution param (in sdl section) to provide support for restricting the actual drawing area to a smaller rectangle than the full size of the window/screen. Possible values are auto or WxH.
  • auto is the current behaviour with no changes to anything whatsoever (the drawing area always fills the screen/window). This is the default to ensure 100% backward compatibility.

  • If the max resolution is specified in WxH format, no window size refinement is performed (except for ensuring a minimum window size of 640x480). Instead, the refinement is applied to the viewport_resolution rectangle. The resulting draw area is always centered to the window/screen.

  1. Introduce new window_position param (in sdl section) to set the initial window position. Possible values are X,Y, where X and Y are the desired window coordinates, or auto, which is the current behaviour (leave it to the OS). Defaults to auto for backward compatibility.

  2. Introduce new window_decorations param (in sdl section) to show/hide the window decorations. Defaults to true for backward compatibility.

Config examples

This is how the config would look like for case 1:

[sdl]
fullscreen = true
viewport_resolution = 960x720

And for case 2:

[sdl]
viewport_resolution = 1060x1080
window_position = 0,0
window_decorations = false

Notes

  • I've tested it on Windows 10 with all sorts of permutations (start in fullscreen, start in windowed, switch between the two, games that change resolutions, different shaders, etc)
  • Only opengl is supported currently. Support could (and should) be added for texture output as well. I might do that later, or someone else can pick it up, but I just wanted to show this to you guys first, I've spent enough time on it already.
  • surface output changes the window size when the games change the resolution, so conceptually is a bit at odds with the viewport_resolution option. But honestly, I don't care, as my understanding is that surface is planned to be removed soon (rightly so).
  • I've done some cleanup in separate commits, if you don't like them, I can delete them.

@kcgen kcgen self-requested a review September 18, 2021 15:12
@kcgen kcgen added the enhancement New feature or enhancement of existing features label Sep 18, 2021
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
src/gui/sdlmain.cpp Outdated Show resolved Hide resolved
Copy link
Member

@kcgen kcgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the feature @johnnovak , and the code looks great.
Just a couple minor comments.

The only blocker to merge is getting Texture support in - this isn't urgent, and hopefully you can progress it when time permits.

We will deliberately avoid any changes to sdmain and related video areas while you progress it.

I'm hoping it can be worked such a way that all your general functions are just that: agnostic of output method, and then the resulting values are used in those output-specific routines.

@johnnovak
Copy link
Member Author

johnnovak commented Sep 18, 2021

Love the feature @johnnovak , and the code looks great.
Just a couple minor comments.

Cheers, and thanks for the review! Haven't touched C++ for the last 10 years or so, but I'm getting up to speed quickly, plus I'm learning a few interesting things about the new C++ standard in the process :)

The only blocker to merge is getting Texture support in - this isn't urgent, and hopefully you can progress it when time permits.

Yeah I agree that is necessary, I just wanted to show what I've got before I spend even more time on the PR (to confirm we are in agreement about the feature in general).

We will deliberately avoid any changes to sdmain and related video areas while you progress it.

I very much appreciate that. It shouldn't be too hard, I expect I will get it done during the next week sometime.

I'm hoping it can be worked such a way that all your general functions are just that: agnostic of output method, and then the resulting values are used in those output-specific routines.

We are in agreement :)

@kcgen
Copy link
Member

kcgen commented Sep 20, 2021

All sounds good, @johnnovak!

@kcgen
Copy link
Member

kcgen commented Sep 21, 2021

This is coming along great @johnnovak !
Are texture modes ready to go?

Couple housekeeping steps:

I've sent an invite to join the project with write access:

2021-09-21_05-55

It's a couple days old now, but the invite should be in your email -- if not I can re-send. This will let you create branches directly in the repo as a contributor and have the CI chain run for you. (Right now, I can only authorize the 3rd party analyzers.)

Once accepted, you can push your branch from your fork into a local repo:

Grab a fresh clone of the repo:

git clone --recurse-submodules git@github.com:dosbox-staging/dosbox-staging

Now cd back to your forked directory and push the branch into the new clone:

cd /back/to/forked/repo
git push /path/to/cloned/dosbox-staging \
    jnovak/render-resolution-and-window-improvements-final

Now you can work off the main repo and git push your branch (fewer hops and directly-drive CI) 🚀

@johnnovak
Copy link
Member Author

johnnovak commented Sep 22, 2021

Thanks, I missed the invite because I get tons of emails to my github folder every day at work, so it's easy to miss a few notifications now and then...

I've followed your instructions and have pushed up my branch again:
#1280

Status update: I'm just finished with adding support for texture mode. I'll spend some time this evening giving the whole thing another round of testing and then it's ready to go.

@kcgen
Copy link
Member

kcgen commented Sep 23, 2021

(merged as PR#1280)

@kcgen kcgen closed this Sep 23, 2021
@kcgen kcgen added this to the 0.78 release milestone Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or enhancement of existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants