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 option to improve frame pacing through duplicate frames if below 60hz. #12602

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Jan 29, 2020

Should help #9736, and partially papers over #12325, and should help #12460 .

@hrydgard hrydgard requested a review from LunaMoo January 29, 2020 09:17
@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Jan 29, 2020
@hrydgard hrydgard added this to the v1.10.0 milestone Jan 29, 2020
Copy link
Collaborator

@LunaMoo LunaMoo left a comment

Choose a reason for hiding this comment

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

Seeing iota97 #12325 (comment) it seems disabling the flag on unthrottle might not be enough:c.

I wonder, maybe the actual problem causing this was also causing the bug with recording from "Current Framebuffer" in games running under 60fps as it looked similar ~ recorded old frames every other one.

@@ -345,6 +345,11 @@ void GameSettingsScreen::CreateViews() {
#ifdef _WIN32
graphicsSettings->Add(new CheckBox(&g_Config.bVSync, gr->T("VSync")));
#endif
CheckBox *frameDuplication = graphicsSettings->Add(new CheckBox(&g_Config.bRenderDuplicateFrames, gr->T("Render duplicate frames to 60hz")));
frameDuplication->OnClick.Add([=](EventParams &e) {
settingInfo_->Show(gr->T("RenderDuplicateFrames Tip", "Can make framerate smoother in games that run at 30hz"), e.v);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe ".. in games that run under 60hz"? Since it will also affect 20 and other weird fps PSP games have.

@hrydgard
Copy link
Owner Author

Yeah, there still seems to be some kind of gremlin in the machinery related to this...

@mirh
Copy link

mirh commented Jan 29, 2020

I know that 60hz is the standard refresh rate, but perhaps tooltips should be a bit more "general" than that.

@hrydgard
Copy link
Owner Author

Yeah, fair enough. Changed it again.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Jan 29, 2020

It's whatever, but to make it clear. 60hz here came from about PSP being limited to max 60hz and the option affecting only things < 60hz. This option is not going do display differently on modern displays than it would on 60hz ones.

If PPSSPP will ever allow more ie to support for example 120 fps patches, it might then differ, but currently it's not and most games would break horribly when patched to such framerates even if working fine with 60fps patch(and that would already be limited, since most of those patches break stuff).

@hrydgard
Copy link
Owner Author

I think mirh was referring more to people maybe not knowing that 60hz is the standard PSP refresh rate. Though then again, if you don't know that, you might not be interested in the option anyway. Not sure which wording makes more sense.

@iota97
Copy link
Contributor

iota97 commented Jan 29, 2020

I think it's better to say something like: "game with frame rate capped below 60"

Saying running lower might end up with user enabling this when the HW can't keep up with 60 fps. Not sure if it may help there as well tho'.

@mirh
Copy link

mirh commented Jan 29, 2020

I was actually thinking to the computer display...
If for whatever reason I'm natively running at 30hz (which could also happen during normal operation with some variable refresh ratio monitor?), duplicating frames doesn't make sense regardless.

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

This may also have issues with alt speed over 100%, since that's partially unthrottled. Maybe when alt speed is above 100% we auto-disable?

Also, created #12665 for the flickering issue.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Mar 1, 2020

Rebased on master, resolving the merge conflict. Will merge after CI is green, I think.

@hrydgard
Copy link
Owner Author

hrydgard commented Mar 15, 2020

Rebased, merging shortly. I don't see much of an interesting improvement myself, but it might trigger various devices and drivers into better behavior if lucky I guess. as unknown mentions in #12101. So let's merge.

@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Mar 15, 2020

Also, this fixes #12460 right (by way of an option, at least)?

-[Unknown]

@hrydgard
Copy link
Owner Author

Yeah, changed the initial post so it'll get linked.

@hrydgard hrydgard merged commit eeff56c into master Mar 15, 2020
@hrydgard hrydgard deleted the frame-duplication branch March 15, 2020 17:33
if (shaderInfo && g_Config.iRenderingMode != FB_NON_BUFFERED_MODE)
postEffectRequiresFlip = shaderInfo->requires60fps;
postEffectRequiresFlip = (shaderInfo->requires60fps || g_Config.bRenderDuplicateFrames) && !(g_Config.bFrameSkipUnthrottle && !FrameTimingThrottled());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops, the setting only applies if shaderInfo is not nullptr.

-[Unknown]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants