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

Remove Frameskip #4322

Merged
merged 1 commit into from Oct 8, 2016
Merged

Remove Frameskip #4322

merged 1 commit into from Oct 8, 2016

Conversation

Helios747
Copy link
Contributor

@Helios747 Helios747 commented Oct 8, 2016

Frameskip is really broken in Dolphin and graphics / JIT devs say fixing it would be a nightmare. So I'm removing it to protect the user. We don't even save the setting to disk anyways.

It's broken in that it's very inconsistent in what it even does to games. Sometimes it does nothing at all. Sometimes it just renders black frames in place of the frames it drops. Sometimes it entirely breaks the game. JMC says it doesn't often do what it's supposed to.

Can somebody who knows frame advance code look over this to make sure I didn't delete something important, whoever did the frame advance stuff named some functions and configmanager stuff "Frameskip" for some reason.

@degasus, please review the fifo stuff. I had no idea what I was doing.


This change is Reviewable

@Helios747 Helios747 changed the title I hate features Remove Frameskip Oct 8, 2016
@Helios747 Helios747 force-pushed the I_hate_features branch 12 times, most recently from 69425b5 to 9798299 Compare October 8, 2016 04:10
@degasus
Copy link
Member

degasus commented Oct 8, 2016

The VideoCommon changes LGTM


Reviewed 18 of 18 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


Source/Core/DolphinWX/Frame.cpp, line 310 at r1 (raw file):

EVT_MENU_RANGE(IDM_SAVE_SLOT_1, IDM_SAVE_SLOT_10, CFrame::OnSaveState)
EVT_MENU_RANGE(IDM_SELECT_SLOT_1, IDM_SELECT_SLOT_10, CFrame::OnSelectSlot)
EVT_MENU_RANGE(IDM_FRAME_SKIP_0, IDM_FRAME_SKIP_9, CFrame::OnFrameSkip)

May you also drop the globals IDM_FRAME_SKIP_ ?


Source/Core/VideoCommon/Fifo.cpp, line 89 at r1 (raw file):

  }

  p.Do(s_skip_current_frame);

Did you increase the savestate version?


Source/Core/VideoCommon/Fifo.h, line 47 at r1 (raw file):

bool AtBreakpoint();
void ResetVideoBuffer();
void SetRendering(bool bEnabled);

Please also remove the header of SetRendering


Comments from Reviewable

@Helios747
Copy link
Contributor Author

Review status: all files reviewed at latest revision, 3 unresolved discussions.


Source/Core/DolphinWX/Frame.cpp, line 310 at r1 (raw file):

Previously, degasus (Markus Wick) wrote…

May you also drop the globals IDM_FRAME_SKIP_ ?

Done.

Source/Core/VideoCommon/Fifo.cpp, line 89 at r1 (raw file):

Previously, degasus (Markus Wick) wrote…

Did you increase the savestate version?

Done.

Source/Core/VideoCommon/Fifo.h, line 47 at r1 (raw file):

Previously, degasus (Markus Wick) wrote…

Please also remove the header of SetRendering

Done.

Comments from Reviewable

@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

  • rs3-bumpmapping on dx-win-nv: diff

automated-fifoci-reporter

@degasus
Copy link
Member

degasus commented Oct 8, 2016

:lgtm:


Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@degasus degasus merged commit a86b2c1 into dolphin-emu:master Oct 8, 2016
@escape209
Copy link
Contributor

And just like the codes that increase frame-rate, it should be possible to make frame-rate reduction codes. I made an experimental one for Burnout 2, for instance. Should be far less annoying than frame-skip and more flexible.

@Helios747 Helios747 deleted the I_hate_features branch September 20, 2017 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants