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

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

Open
chloebangbang opened this issue Sep 27, 2021 · 4 comments · May be fixed by #275
Open

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

chloebangbang opened this issue Sep 27, 2021 · 4 comments · May be fixed by #275

Comments

@chloebangbang
Copy link
Contributor

at 4:3 resolutions, camera dimensions are as expected
image

but at non-4:3 resolutions, the camera dimensions are noticeably wrong, with 1600x900, 1440x900, and 1024x600 being the most egregious
image

Renderer.base_height seems to expect 240 pixels exactly, and going any higher extends the camera, but, given that no common 16:9 resolutions are divisible by 240, that's gonna require Renderer.scale to be a float before this is fixable

@Bonicgamer
Copy link
Contributor

Would using SDL_RenderSetLogicalSize help here? I found out about it yesterday and I thought it could be useful here if we don't care about base_height being just a couple off of 240, and it would get rid of the need for Renderer.scale.

For instance, 16:9 would be base_height: 243, base_width: 432, and SDL_RenderSetLogicalSize: {w: base_width h: base_height}, if I'm thinking about it correctly.

@isage
Copy link
Collaborator

isage commented Nov 24, 2021

SDL scaling is meh. Results aren't good and sharp, especially with text.
We could try, though, maybe something changed

@xordspar0
Copy link
Contributor

Actually, I tried exactly what @Bonicgamer suggested and it's quite nice. The scaling is nice and crisp, the viewport height doesn't change based on the resolution, and the renderer code is a lot simpler because it can just focus on two resolutions: 320x240 and 432x243.

I'm going to test it a bit more before submitting a PR, but my initial tests look really good.

image

@xordspar0
Copy link
Contributor

There are still some minor issues in my PR that need to be worked out, but anyone can try it out now: #275

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 a pull request may close this issue.

4 participants