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

VideoCommon: VI Skip #11367

Merged
merged 1 commit into from Jan 19, 2023
Merged

VideoCommon: VI Skip #11367

merged 1 commit into from Jan 19, 2023

Conversation

Sam-Belliveau
Copy link
Contributor

@Sam-Belliveau Sam-Belliveau commented Dec 23, 2022

This hack uses the improved pacing branch to look at the amount of time ahead / behind of the timings the emulation is. If the emulation is more that 40ms behind the timings (length of the audio buffer) then it will skip sending an VI to the CPU in order to prevent the game from requesting to draw a new frame. While this is "hacky," it is the most compatible way to induce a frame skip when GPU limited and is very non invasive compared to the old frame skip.

Here is an image to help visualize the timings:
image

This image is old so the only thing incorrect is the size of the buffer, which is now the same as TimingVariance, which is 40ms.

Heres a video of an old version. The audio is full speed but the video is not: VIDEO

@Sam-Belliveau Sam-Belliveau changed the title Make Lag Go Away :) ExperimentalVISkip Dec 23, 2022
@Sam-Belliveau Sam-Belliveau force-pushed the lagbegone branch 11 times, most recently from 7bf05ef to a1de310 Compare December 28, 2022 00:53
@Sam-Belliveau Sam-Belliveau marked this pull request as ready for review December 28, 2022 00:53
@JosJuice
Copy link
Member

Could we automatically disable this when Core::WantsDeterminism returns true? Because as far as I can tell, there's no way this is going to work with netplay/TASing.

@JMC47
Copy link
Contributor

JMC47 commented Dec 28, 2022

Edit: MKWii TASers don't need determinism, ignore me.

@Sam-Belliveau Sam-Belliveau changed the title ExperimentalVISkip VideoCommon: VI Skip Jan 5, 2023
@Sam-Belliveau Sam-Belliveau force-pushed the lagbegone branch 2 times, most recently from 71a432f to c063d56 Compare January 6, 2023 21:25
@Sam-Belliveau Sam-Belliveau force-pushed the lagbegone branch 4 times, most recently from a1c5094 to 1adf419 Compare January 6, 2023 22:25
@Sam-Belliveau Sam-Belliveau force-pushed the lagbegone branch 4 times, most recently from 19e3c52 to a1d0275 Compare January 9, 2023 16:07
@phire
Copy link
Member

phire commented Jan 12, 2023

If you want to do some more experimentation:

Alternative idea one:

Instead of outright skipping the VI interrupt, compatibility might be higher if you simply switch to an uneven frame pacing. Instead of two 16ms frames, have one frame that's 31ms and the other which is 1ms. That way the total number of VI interrupts stays the same, but the short frame is (hopefully) short enough that it always triggers the game's built in frame limiter.

Alternative idea two:

Instead of moving VI interrupts, maybe it's smarter to move the final finished interrupt after the EFB to XFB copy until after the VI interrupt. Dolphin would still see the finished frame on-time (if the immediate present xfb option is enabled), but the game would think the GPU is busy and not start work on the next frame.

@Sam-Belliveau Sam-Belliveau force-pushed the lagbegone branch 2 times, most recently from 9663b55 to 4065173 Compare January 13, 2023 17:00
@dvessel
Copy link
Contributor

dvessel commented Jan 13, 2023

Just a general indicator on the overhead with the recent changes comparing latest master vs lagbegone rebased to latest master.

The test simply runs F-Zero Sand Ocean intro as fast as possible at 1x resolution.

master 5.0-18221-f4f9439-f4f9439

11436 frames 4.852ms, v-blanks 4.684ms - metal pass 1
11436 frames 4.841ms, v-blanks 4.673ms - metal pass 2
11436 frames 4.842ms, v-blanks 4.674ms - metal pass 3
34308 frames 4.845ms, v-blanks 4.677ms complete

lagbegone 5.0-18226-2756110-f4f9439

11436 frames 4.834ms, v-blanks 4.665ms - metal pass 1
11436 frames 4.876ms, v-blanks 4.707ms - metal pass 2
11436 frames 4.882ms, v-blanks 4.712ms - metal pass 3
34308 frames 4.864ms, v-blanks 4.695ms complete

It previously had an overhead of about 0.4ms. It’s now a fraction of that.

options set in the terminal

@delroth
Copy link
Member

delroth commented Jan 19, 2023

🎉

@delroth delroth merged commit 8d5edb1 into dolphin-emu:master Jan 19, 2023
11 checks passed
const DT max_fallback =
std::chrono::duration_cast<DT>(DT_ms(Config::Get(Config::MAIN_TIMING_VARIANCE)));
std::chrono::duration_cast<DT>(DT_ms(Config::Get(Config::MAIN_MAX_FALLBACK)));
Copy link
Contributor

Choose a reason for hiding this comment

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

This change from 40ms to 100ms causes https://bugs.dolphin-emu.org/issues/13166 (in Dual Core mode).

Now... Dual Core is notorious for being weird, but why exactly was this changed here anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

having it at 40ms means that 40ms of lag causes slowdown, which stops vi skip from running at full speed if the fps is < 25. I didn't want to make the audio latency 40ms so i didn't increase the variance. i don't know why increasing this would cause it to hang as the throttler is independent of the emulated system, and any changes to the throttler should have no effect on the rest of the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Remember that Dual Core leaks external state into the emulation. If the larger fallback allows the GPU thread to get further ahead or behind the CPU thread then that's observable by a game in Dual Core mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is the solution here really to lower the fallback? A higher fallback allows the emulator to run at full speed even during stutters which is a huge bonus.

@delroth
Copy link
Member

delroth commented Feb 12, 2023 via email

@Anuskuss
Copy link

Anuskuss commented Feb 13, 2023

What do you guys think about making this the default? My PC is powerful enough where I don't need this but even I experience lag sometimes. AFAICT this is unlikely to have any negative side effects (unlike traditional frame skip) other than being incompatible with a few games. It might actually be the way it works on real hardware given the fact that some games choose to ignore the VBLANK altogether.

Edit: Also seems like the performance overlay used VPS rather than FPS to determine the speed% which in turn always shows 100% with this option enabled (as of the latest beta).

@cobalt2727
Copy link

That's mentioned in the progress report - it doesn't work in every case.

@Anuskuss
Copy link

That's mentioned in the progress report - it doesn't work in every case.

If it doesn't work does it cause harm? And if it does cause harm can't you just blacklist it? Unless it causes subtle issues then of course it should be considered a (speed) hack and default to off but to me it doesn't look like that's the case.

@OatmealDome
Copy link
Member

OatmealDome commented Feb 13, 2023

I wouldn't be comfortable with making a hack like this enabled by default. If VBI Skip causes some game we haven't tested to break, and an newbie user tries to play it and finds that it doesn't work properly, that's a poor first experience with Dolphin. They shouldn't be expected to know to turn off a random checkbox buried in a settings menu.

So far (according to the progress report), games broken with VBI Skip tend to just freeze on a black screen or lag even worse than they already do.

@Anuskuss
Copy link

that's a poor first experience with Dolphin

I'd argue the opposite: Thousands of (Android) users running their favorite game only for it to lag and stutter is a worse experience (when I doesn't have to be). But I don't know, maybe keeping it off is the safer approach; I just don't see what issues it could cause.

I'd say defaulting it to on (between betas) may be enough to catch all misbehaving games.

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