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

add vblank_ntsc_fixup to UI #12042

Merged
merged 5 commits into from May 30, 2022
Merged

add vblank_ntsc_fixup to UI #12042

merged 5 commits into from May 30, 2022

Conversation

Darkhost1999
Copy link
Contributor

Follow up on #11070

GUI work can be done later, it's a not a merge blocker.

It is later now. Essentially I'm just adding VBlank NTSC Fixup to QT
image
It is a vblank setting so I figured putting them together is logical. Can be moved

@Megamouse
Copy link
Contributor

It's really weird that the group box and the option have different casing

@Darkhost1999
Copy link
Contributor Author

Ah I see VBlank not Vblank.

@Darkhost1999
Copy link
Contributor Author

Better? Is the location fine? The option defaults to false and works on my machine.
I'm still curious how many games are effected by this and hope exposing to the UI encourages testing.

@YuriNator557
Copy link

As far as I am aware there is only 1 game that is affected by this?

@capriots
Copy link
Contributor

It doesn't affect anything negatively, right? I would have it active by default since it's more accurate.
I wouldn't even add it to the GUI. Is there a reason to ever deactivate it?

@Darkhost1999
Copy link
Contributor Author

That's a really good question ngl.
https://www.playstation.com/en-us/support/hardware/manuals/#PS3
Approximately page 16 for every PS3 instruction manual says they output video in Standard NTSC video output.
Honestly this being the default global setting sounds more real to a PS3 console.

But that just encourages exposing the setting to the UI for more testing and feedback.

@kd-11
Copy link
Contributor

kd-11 commented May 21, 2022

It doesn't affect anything negatively, right? I would have it active by default since it's more accurate. I wouldn't even add it to the GUI. Is there a reason to ever deactivate it?

Yes. Most monitors are 60.00 not 59.94. Framepacing issues.

@capriots
Copy link
Contributor

capriots commented May 21, 2022

I don't think that's true at all. I have two different PC monitors and neither one exposes a 60.00Hz mode. They both run at 59.951Hz. My laptop runs at 59.986Hz. Also, the official HDMI Licensing actually mandates 59.94Hz support (see pages 101+102 https://engineering.purdue.edu/ece477/Archive/2012/Spring/S12-Grp10/Datasheets/CEC_HDMI_Specification.pdf).

@Darkhost1999
Copy link
Contributor Author

Darkhost1999 commented May 22, 2022

These other PRs got me a little curious.
For example: If the original issue was
#11039 which introduced the NTSC fixup setting that allowed synchronizing with the 59.94 framerate of the game.
Then does #12060 change that synchronization behavior so as the NTSC fixup becomes obsolete?

I mean if 59.94 is more accurate to the PS3 then the PS3 Native framelimiter setting will just handle the 59.94 function/behavior if someone wants true PS3 performance right?

So this PR then becomes obsolete in the process.

@elad335
Copy link
Contributor

elad335 commented May 22, 2022

I've checked and the game configures the VBlank rate to "Whatever rate the video is outputting" and not fixed 59.94. As if it's convinced the current display rate is 59.94 already. In short, you still need this setting.

@capriots
Copy link
Contributor

How about instead of a slider that can only select integers + a checkbox for proper NTSC timing, we use a textbox for VBlank that allows floats and defaults to 59.94, like in PCSX2.
Screenshot 2022-05-22 173551
That would also allow users with weird refresh rates to fix potential framepacing issues with games that use VBlank to cap framerate.

@elad335
Copy link
Contributor

elad335 commented May 22, 2022

What kind of weird refresh rates?

@capriots
Copy link
Contributor

Well like I said, my monitors run at 59.951Hz for example and my laptop at 59.986Hz. You can check it in Windows display settings or at https://www.vsynctester.com/.

@Darkhost1999
Copy link
Contributor Author

Just a trim and a wave. And like this?
image

@capriots
Copy link
Contributor

Yes

@elad335
Copy link
Contributor

elad335 commented May 22, 2022

Even if it's implemented I would like to keep this setting, because 59.94 is not the real number it is 60 * 1000 / 1001.

@Darkhost1999
Copy link
Contributor Author

Darkhost1999 commented May 22, 2022

Yes. Changing the UI is easy. But the code that goes into it is harder. I can show the idea. But I don't know if I could actually implement a doublespinbox like that.
Besides the slider and checkbox makes more sense in this use case.

@capriots
Copy link
Contributor

How about separate numerator and denominator settings? Like this
Screenshot 2022-05-22 182448
From what I can tell that should be easy to implement and allows to adjust the frequency with high precision.

@elad335
Copy link
Contributor

elad335 commented May 22, 2022

That should be ok.

@Megamouse
Copy link
Contributor

this looks so overly complicated and noone will understand it

@Darkhost1999
Copy link
Contributor Author

I'm leaving this as is so as not to overcomplicate things.
Unless are there any other concerns?

It seems maybe add a warning -> May cause frame pacing issues if used incorrectly?

@capriots
Copy link
Contributor

capriots commented May 23, 2022

It seems maybe add a warning -> May cause frame pacing issues if used incorrectly?

Nah, if someone is using a 59.94Hz display it would actually solve frame pacing issues. And it would only introduce frame pacing issues if you are using 60.00Hz. If a display is running any other refresh rate it would only barely make it worse or better, depending on what is closer to your display. And it doesn't even matter for games that don't use VBlank to cap fps, then you just need to activate Vsync and you would be fine. It is really complicated. To really solve it for every user, you would have to be able to adjust the frequency more precisely. I'll open an issue about it later.

I still think it should be active by default and I think it should be communicated that it is more accurate, since a PS3 is always running at 59.94Hz outside of playing PAL content like PS1/PS2 games and DVDs/Blu-Rays.

@Megamouse Megamouse merged commit e4fe335 into RPCS3:master May 30, 2022
@Darkhost1999 Darkhost1999 deleted the Vblank branch May 30, 2022 18:48
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.

None yet

6 participants