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

PCSX2: Scaling Types / 16:9 FMV aspect ratio switch #1918

Closed
wants to merge 3 commits into from

Conversation

FlatOutPS2
Copy link
Contributor

@FlatOutPS2 FlatOutPS2 commented Apr 26, 2017

  • Adds Scaling Types

And adds a combo box(in Config > Emulation Settings > GS Window tab) to
select the scaling type.

Adds new "Centered" option to scale the image down to a centered image
at the native(1x) resolution used by the game.

And adds "Integer" option to scale to the maximum integer magnification
of the native image set by the game which fits within the window/screen.

Adds new aspect ratio "Frame" which sets the aspect ratio to that of the
native image used by the game.
Closes #1916

  • Add 16:9 option for FMV aspect ratio switch

Adds combobox option to switch to 16:9 aspect ratio while an FMVs plays to fix
16:9 widescreen FMV's when using custom widescreen patches to play a
game at other aspect ratios than 4:3 or 16:9.

Close: #2114

@gregory38
Copy link
Contributor

gregory38 commented Apr 26, 2017

I'm not sure I love the pixel name.

However, I'm not sure it is a good idea to query the size on the GS plugin. I think you can read the CRTC related registers on the EE side directly. However it might duplicate some code for the height. I'm not sure what is the best solution.

@FlatOutPS2
Copy link
Contributor Author

I'm not sure I love the pixel name.

It's called pixel aspect ratio, so I'm not sure what would be better.

However, I'm not sure it is a good idea to query the size on the GS plugin. I think you can read the CRTC register on the EE side directly. However it might duplicate some code for the height. I'm not sure what is the best solution.

Querying it seemed like the best way to me. Duplicating the code would also mean having to modify the code in both places when improving the CRTC detection.

@ssakash
Copy link
Member

ssakash commented Apr 26, 2017

However, I'm not sure it is a good idea to query the size on the GS plugin. I think you can read the CRTC register on the EE side directly.

Please don't confuse people by adding new names for registers. :P

Though yes, DISP and DISPFB are privileged registers. You can get the values directly from EE and I'd personally prefer if it's done that way to be honest, would remove the unnecessary plugin dependency. It's always ideal to keep most of the code in the core and rely on other modules only when necessary,

Querying it seemed like the best way to me. Duplicating the code would also mean having to modify the code in both places when improving the CRTC detection.

Unless I'm wrong, your plugin side implementation won't work when using a newer version of PCSX2 along with an older version of GSdx plugin. Implementing the function in core instead would not introduce any such problem.

@ssakash
Copy link
Member

ssakash commented Apr 26, 2017

However, I'm not sure it is a good idea to query the size on the GS plugin. I think you can read the CRTC register on the EE side directly.

Okay, apologies. I see what you mean now, you mean all the registers which modify the settings of the PCRTC circuit, right? I was just confused thinking that you meant "one single register" as the CRTC register, my bad.

@gregory38
Copy link
Contributor

Yes I mean all privileged registers that are used to set the size.

I'm afraid there isn't an ideal solution. I let @turtleli decide here 👍

By the way, I'm not sure 1:1 ratio is a good thing. A 512x448 game will be very very small on a 4K display well there is upscaling. Dunno @rz5 do you prefer 1:1 or a multiple that can still fit in the screen (or both option)?

@gregory38
Copy link
Contributor

Actually for upscaling you need to query the size on the GS plugin.

@MrCK1
Copy link
Member

MrCK1 commented Apr 26, 2017

Isn't it possible that this would also break savestates?

@ssakash
Copy link
Member

ssakash commented Apr 26, 2017

Actually for upscaling you need to query the size on the GS plugin.

Looking at the code (not on my IDE, at Github online editor and I could be potentially wrong), @FlatOutPS2 is only sending the native resolution size to core, not the upscaled resolution size.

@willkuer
Copy link
Contributor

willkuer commented Apr 26, 2017

I think the original idea of #1916 is to round the scaling to an integer value such that one pixel of the ps2 output covers an integer number of pixels on the final screen. (Like instead of 512x448 you send a twice larger output 1024x896 where each ps2 pixel covers 4 LCD pixels) Like that you avoid the issue of having a too small output depending on the screen resolution.

@FlatOutPS2
Copy link
Contributor Author

FlatOutPS2 commented Apr 26, 2017

By the way, I'm not sure 1:1 ratio is a good thing. A 512x448 game will be very very small on a 4K display well there is upscaling.

You can use zoom to increase the size when using the native size(set zoom to 200% and you get 1024x896), but the pixel aspect ratio will work like a regular aspect ratio(using the full width or height of the screen) too.

@mirh
Copy link

mirh commented Apr 26, 2017

By the way.. perhaps it's just i'm not a native speaker, I dunno.. But "aspect ratio" tag for something that is a point feels wrong.

@turtleli
Copy link
Member

I'm not sure whether #1916 is requesting integer scaling or unscaled. Personally I see integer scaling (as willkuer described) to be the more useful option - I don't think unscaled will be nice on high res monitor (as gregory already mentioned).

I let turtleli decide here

Can I decide to not decide? ;) I think querying GSdx is probably the better option since it avoids code duplication. The other way would be to merge GSdx into PCSX2... XD

if (g_Conf->GSWindow.ScaleToImageSize && image_width > 0 && image_width < client.GetWidth() &&
image_height > 0 && image_height < client.GetHeight() && g_Conf->GSWindow.AspectRatio != AspectRatio_Stretch) {
viewport.x = image_width;
viewport.y = (int)((double)image_width / targetAr);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -615,6 +623,22 @@ void GSFrame::OnUpdateTitle( wxTimerEvent& evt )
out << std::fixed << std::setprecision(2) << fps;
OSDmonitor(Color_StrongGreen, "FPS:", out.str());

int new_width, new_height;
GSgetImageSize(&new_width, &new_height);
if (new_width > 0 && new_height > 0)

This comment was marked as spam.

if (gsPanel->image_width != new_width || gsPanel->image_height != new_height)
{
gsPanel->image_width = (int)new_width;
gsPanel->image_height = (int)new_height;

This comment was marked as spam.

zoom = std::max( (float)arr, (float)(1.0/arr) );

viewport.Scale(zoom, zoom*g_Conf->GSWindow.StretchY.ToFloat()/100.0 );
if (viewport == client && EmuConfig.Gamefixes.FMVinSoftwareHack && g_Conf->GSWindow.IsFullscreen)
viewport.x += 1; //avoids crash on some systems switching HW><SW in fullscreen aspect ratio's with FMV Software switch.
viewport.x += 1; // avoids crash on some systems switching HW><SW in fullscreen aspect ratio's with FMV Software switch.

This comment was marked as spam.

This comment was marked as spam.

@@ -48,6 +48,9 @@ class GSPanel : public wxWindow
bool m_coreRunning;

public:
int image_width;
int image_height;

This comment was marked as spam.

@@ -947,6 +947,15 @@ EXPORT_C GSgetTitleInfo2(char* dest, size_t length)
strcpy(dest, s.c_str());
}

EXPORT_C GSgetImageSize(int *image_width = 0, int *image_height = 0)
{
if (gsopen_done && s_gs != NULL)

This comment was marked as spam.

@@ -947,6 +947,15 @@ EXPORT_C GSgetTitleInfo2(char* dest, size_t length)
strcpy(dest, s.c_str());
}

EXPORT_C GSgetImageSize(int *image_width = 0, int *image_height = 0)

This comment was marked as spam.

This comment was marked as spam.

@FlatOutPS2
Copy link
Contributor Author

FlatOutPS2 commented Apr 26, 2017

I'm not sure whether #1916 is requesting integer scaling or unscaled. Personally I see integer scaling (as willkuer described) to be the more useful option - I don't think unscaled will be nice on high res monitor (as gregory already mentioned).

As I said integer scaling is possible with my implementation by using zoom. With integer scaling setting Internal Resolution in GSdx to 2x native(512x448) in GSdx will only result in 1024x896. However, with my implementation you can set the IR to 4x native and zoom to 200%. This will also result in 1024x896, but with at a higher resolution.

@rz5
Copy link
Contributor

rz5 commented Apr 27, 2017

By the way, I'm not sure 1:1 ratio is a good thing. A 512x448 game will be very very small on a 4K display well there is upscaling. Dunno @rz5 do you prefer 1:1 or a multiple that can still fit in the screen (or both option)?

@gregory38 On second thought, yeah, I want both! I want to be able to see the real size, no matter how tiny it may be and I want PCSX2 to use the largest integer multiple resolution that can fit inside the window.

I think that would require two checkboxes to accomplish it. "Stretch to fit" and one underneath it called "Integer scaling"

Right now it's as if PCSX2 always has "Stretch to fit" on and "Integer scaling" off. If I disable "Stretch to fit", I should get the real size, tiny as it is. If "Stretch to fit" is enabled along with "Integer scaling", then the picture should be stretched to the largest integer multiple that can still fit the window.

And I guess down the line you should have a "Keep aspect ratio" checkbox too, so you can disable it and allow a "Stretch to fill" mode where the picture is stretched to fill the whole screen. I wouldn't use it but there are savages out there, I dunno.

Also @FlatOutPS2 - thanks for working on these commits

@FlatOutPS2
Copy link
Contributor Author

FlatOutPS2 commented Apr 27, 2017

@rz5 This PR currently offers all the possibilities you mention...

@rz5
Copy link
Contributor

rz5 commented Apr 27, 2017

No offense, but I don't want to play around with the Zoom option, as versatile as it is.

@tony971
Copy link

tony971 commented Apr 27, 2017

Sounds like mednafen's .stretch setting

https://mednafen.github.io/documentation/#Global+Settings+Reference

@rz5
Copy link
Contributor

rz5 commented Apr 27, 2017

Sounds like mednafen's .stretch setting

@tony971 Yup, like that and like RetroArch's "Integer scaling" video option. I'm not sure what's the reasoning behind that aspect_mult2 option tho...

@tony971
Copy link

tony971 commented Apr 27, 2017

http://forum.fobby.net/index.php?t=msg&&th=938&goto=3141#msg_3151

The default stretch of "aspect_mult2"(specifically the mult2 bit) is intended to allow for interlaced video and/or scanline effects to be displayed properly.

return;

// Disregard the disabled circuit. With full boot the BIOS will fill the disabled circuit with its own data.
int i = en0 && !en1 ? 0 : en1 && !en0 ? 1 : -1;

This comment was marked as spam.

And adds a combo box(in Config > Emulation Settings > GS Window tab) to
select the scaling type.

Adds new "Centered" option to scale the image down to a centered image
at the native(1x) resolution used by the game.

And adds "Integer" option to scale to the maximum integer magnification
of the native image set by the game which fits within the window/screen.

Adds new aspect ratio "Frame" which sets the aspect ratio to that of the
native image used by the game.
Adds option to switch to 16:9 aspect ratio while an FMVs plays to fix
16:9 widescreen FMV's when using custom widescreen patches to play a
game at other aspect ratios than 4:3 or 16:9.

Close: PCSX2#2114
Adds checkbox in GS Window of Emulation Settings.

When using Centered or Integer Scaling Types or "Frame"Aspect Ratio, this displays the image as the true height (half the normal height) for games that use the "Frame" interlacing mode.
@lightningterror
Copy link
Contributor

What's the status on this, is it ready for a merge ?

@FlatOutPS2
Copy link
Contributor Author

What's the status on this, is it ready for a merge ?

Yeah, I updated with the suggestion by @ssakash. It should be ready.

@James-F2
Copy link

May I suggest an important feature before it gets merged?

This is not related to integer scaling but to pixel accurate blur-free 4:3 scaling.
512x480 is quite low resolution and it's not 4:3, so when it gets upscaled to 4:3 (on a 1920x1080 screen) with bilinear filtering the result is quite blurry.
I often use in other emulators a simple Scale x3,4,5 filter BEFORE bilinear filtering, thus the filtering is doing down-scaling and the resulting image is much sharper than upscaling as the interpolated pixels only where they need to be.
This method is called "super sampling" which is simple yet very effective for uneven scaling with the sharpest result, yet very close to being pixel perfect.

@FlatOutPS2,
Do you think a scaling option like this is relevant to PCSX2?

@FlatOutPS2
Copy link
Contributor Author

Do you think a scaling option like this is relevant to PCSX2?

It's relevant to PCSX2, but it's out of scope for this PR. ;) Did you try disabling "Texture Filtering of Display" (in the GSdx Shader Configuration window) to reduce the blur?

@James-F2
Copy link

Understood, I though you might kill 2 birds with one blow since you already worked on more than a few scaling features on this PR.
Yes I am aware of the "texture filtering" feature and I was talking exactly about that.
It is important to have some kind of Interpolation when doing non integer scaling because without it (nearest neighbor) the image is distorted especially with low resolutions like 320x240 or 640x480.
I hoped to reduce some of that interpolation blur of the filtering by upscaling the image prior to it.

@FlatOutPS2
Copy link
Contributor Author

Understood, I though you might kill 2 birds with one blow since you already worked on more than a few scaling features on this PR.

Yes, but if I keep adding features it will never get merged. ;)

I'll have a look at this later, but there are a few other projects that I need to finish first.

@lightningterror
Copy link
Contributor

Yes, but if I keep adding features it will never get merged. ;)

I agree, it's been delayed for almost a year now, other features can be added in separate PRs, let's not delay this any longer and merge it 😛

@MrCK1
Copy link
Member

MrCK1 commented Feb 22, 2018

That's nowhere near as bad as the "endless mountain of code" in #1699 😛

@mirh
Copy link

mirh commented Feb 22, 2018

Nah, I'd say macos PR is even more exploded.
Anyway, why you don't just commit this
And @James-F2 create an issue to further discuss this?

@FlatOutPS2
Copy link
Contributor Author

@turtleli Did you do a complete review last time? The only thing that has been changed since is applying the suggestion by ssakash.

@turtleli
Copy link
Member

turtleli commented Mar 1, 2018

I still haven't really reviewed the strings. I'll try and review tomorrow.

@@ -1,4 +1,5 @@
/*
* Copyright (C) 2010-2018 PCSX2 Dev Team

This comment was marked as spam.


// Disregard the disabled circuit. With full boot the BIOS will fill the disabled circuit with its own data.
// When both circuits are enabled use -1 to get the size of the merged frame rectangle.
int i = (en0 ^ en1) ? en1 : -1;

This comment was marked as spam.

@@ -56,30 +77,55 @@ Panels::GSWindowSettingsPanel::GSWindowSettingsPanel( wxWindow* parent )

m_check_SizeLock = new pxCheckBox( this, _("Disable window resize border") );
m_check_HideMouse = new pxCheckBox( this, _("Always hide mouse cursor") );
m_check_ScalingCompensation = new pxCheckBox(this, _("Disable Scaling Type height compensation"));

This comment was marked as spam.

s_AspectRatio += m_combo_AspectRatio | pxExpand;
s_AspectRatio += Label(_("Custom Window Size:"))| pxMiddle;
s_AspectRatio += s_customsize | pxAlignRight;
s_AspectRatio += Label(_("Scaling Type:")) | pxMiddle;

This comment was marked as spam.

L"Frame: Uses the aspect ratio of the frame size of the game, to keep a 1:1 pixel ratio.\n\n"
L"NOTE: Using the widescreen 16:9 aspect ratio will result in a stretched image unless "
L"widescreen is enabled in-game or through a widescreen patch. "
L"Widescreen is not available for all games."));

This comment was marked as spam.

m_text_Zoom->SetToolTip( pxEt( L"Zoom = 100: Fit the entire image to the window without any cropping.\nAbove/Below 100: Zoom In/Out\n0: Automatic-Zoom-In untill the black-bars are gone (Aspect ratio is kept, some of the image goes out of screen).\n NOTE: Some games draw their own black-bars, which will not be removed with '0'.\n\nKeyboard: CTRL + NUMPAD-PLUS: Zoom-In, CTRL + NUMPAD-MINUS: Zoom-Out, CTRL + NUMPAD-*: Toggle 100/0"
) );
m_combo_AspectRatio->SetToolTip(pxEt(L"Stretch: Stretches the image to the window/screen size.\n\n"
L"4:3: Keeps the aspect ratio to 4:3, the default aspect ratio used by the PS2.\n\n"

This comment was marked as spam.


m_check_ScalingCompensation->SetToolTip(pxEt(L"Don't let Scaling Types compensate(double) the height for games that put out an image "
L"that is half as high as displayed. This will half the height for some games. Only for purists.\n\n"
L"NOTE: Only available when \"Frame\" Aspect Ratio or \"Integer\" or \"Centered\" Scaling Type is selected."));

This comment was marked as spam.


int new_width = 0, new_height = 0;
GSgetImageSize(&new_width, &new_height);
if ( framemode_compensation && new_height > 1 )

This comment was marked as spam.

@lightningterror
Copy link
Contributor

@FlatOutPS2 status on this ? It was reviewed 2 weeks ago.

@lightningterror
Copy link
Contributor

The 16:9 FMV aspect ratio switch commit has been merged to master.

@lightningterror
Copy link
Contributor

I'm removing this from 1.6 milestone, I don't see the rest getting merged anytime soon unless someone takes over.

@lightningterror
Copy link
Contributor

I'll close this in favor of https://github.com/PCSX2/pcsx2/tree/flatout_aspectratio
Someone needs to finish the code.

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

Successfully merging this pull request may close these issues.

Request for 16:9 option during fmv playback. [Feature Request] Integer scaling/Fit to height toggle