Kill Renderer #11522
Merged
Kill Renderer #11522
+5,585
−4,841
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows for fowards declaration
The whole ownership model was getting a bit of a mess, with a some of special cases to deal with. And I'm planning to make it even more complex in the future. So here is some upfront work to convert it over to reference counted pointers.
Texture cache occasionally mutates textures for efficiency. Which is awkward if we want to borrow those textures from texture cache to do something else, such as a graphics debugger, or async presentation on another thread. Content locking provides a way to signal that the contents of a texture cache entry should not change. Texture cache will be forced to use alternative strategies.
Slightly simplifies the upcoming refactor
The name kind of conflicts with a new FrameDumper class
Almost all the virtual functions in Renderer are part of dolphin's "graphics api abstraction layer", which has slowly formed over the last decade or two. Most of the work was done previously with the introduction of the various "AbstractX" classes, associated with texture cache cleanups and implementation of newer graphics APIs (Direct3D 12, Vulkan, Metal). We are simply taking the last step and yeeting these functions out of Renderer. This "AbstractGfx" class is now completely agnostic of any details from the flipper/hollywood GPU we are emulating, though somewhat specialized. (Will not build, this commit only contains changes outside VideoBackends)
Mostly involves moving contents of OGLRender to OGLGfx and OGLConfig
phire
force-pushed
the
KillRendererWithFire
branch
from
0c4adf5
to
668cd08
Compare
Not a good idea to abuse bSupportsPostProcessing
Otherwise you get subtle bugs in vulkan that take hours to track down.
Both DX12 and Vulkan already had one.
They were essentially just pass-though methods
There is this nice VideoConfig file that's perfect for it
Co-authored-by: BhaaL <bhaalsen@gmail.com>
Co-authored-by: Mai <mathew1800@gmail.com>
Co-authored-by: Mai <mathew1800@gmail.com> Co-authored-by: BhaaL <bhaalsen@gmail.com> Co-authored-by: iwubcode <iwubcode@users.noreply.github.com>
Frame duplicate detection was inverted. Huge problem for 60fps games where it would see all frames as "duplicates" and nothing would ever be presented.
Fixes issue with post-processing not working
phire
force-pushed
the
KillRendererWithFire
branch
from
998de79
to
5c1b3ac
Compare
|
Apparently the beta build has already been locked in. I've squashed those fixups, will merge when it passes all checks (and I double-check FIFOCI) |
|
Interesting. Earlier versions of this PR were causing the widescreen heuristic to be "fixed" on FIFO CI. But that's gone now. FIFO CI reports no changes. Guess it was triggered by one of the bugs which I fixed along the way. |
|
With this merged, Android is crashing while booting games. Due to limitations in the debugging tools I have for Android, I don't know where it's crashing, but it's a null pointer dereference. I've bisected the bad commit to one of the following:
|
|
Additional information: It apparently only happens with OpenGL, not with Vulkan. |
|
Courtesy of @t895: |
|
Yep, that fixes it. I'll submit a PR. Thanks! |
Pokechu22
added a commit
to Pokechu22/dolphin
that referenced
this pull request
Apr 2, 2023
This fixes a crash when recording fifologs, as the mutex is acquired when BPWritten calls AfterFrameEvent::Trigger, but then acquired again when FifoRecorder::EndFrame calls m_end_of_frame_event.reset(). std::mutex does not allow calling lock() if the thread already owns the mutex, while std::recursive_mutex does allow this. This is a regression from dolphin-emu#11522 (which introduced the HookableEvent system).
SirMangler
pushed a commit
to SirMangler/dolphin
that referenced
this pull request
Apr 22, 2023
This fixes a crash when recording fifologs, as the mutex is acquired when BPWritten calls AfterFrameEvent::Trigger, but then acquired again when FifoRecorder::EndFrame calls m_end_of_frame_event.reset(). std::mutex does not allow calling lock() if the thread already owns the mutex, while std::recursive_mutex does allow this. This is a regression from dolphin-emu#11522 (which introduced the HookableEvent system).
Hibyehello
pushed a commit
to Hibyehello/dolphin
that referenced
this pull request
Apr 27, 2023
This fixes a crash when recording fifologs, as the mutex is acquired when BPWritten calls AfterFrameEvent::Trigger, but then acquired again when FifoRecorder::EndFrame calls m_end_of_frame_event.reset(). std::mutex does not allow calling lock() if the thread already owns the mutex, while std::recursive_mutex does allow this. This is a regression from dolphin-emu#11522 (which introduced the HookableEvent system).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
This is a massive PR, but 90% of the changes are simply moving functions from one class/file to another
Background
Renderer was originally the abstraction interface for video plugins. All it's methods were virtual.
But after the video plugins were merged into the main Dolphin codebase, it slowly grew out of control.
As the backends were refactored over the years to eliminate duplicated functionality, a lot of that common code simply ended up in RenderBase. And new features would often end up added to RenderBase.
I originally started this refactor due to my work on Async Present, as that needed everything to do with presentation and the ImGui UI moved out of render. But that actually made a massive dent, and I was motivated to try and kill it completely.
What better way to stop it collecting more code over time than killing it altogether.
Where has everything gone?
Initialization and various subobjects
Everything that Renderer owned has been moved into global g_ variables.
Most of the initialisation logic was moved into VideoBackendBase.cpp's InitialisedShared.
Back-ends now don't set any globals except their own.
Presentation
There is a new
Presenterclass that handles taking finished XFBs and drawing them to screen. It also fowards the XFBs to frame dumping and is in charge of drawing the ImGui UI (the actual UI code lives in a newOnScreenUIclass)This class will probably be refactored further as the Async Present work continues.
Frame dumping and screenshots
These have moved to a new
FrameDumperclassGraphics API abstraction.
Almost all the virtual functions in Renderer are part of dolphin's "graphics api abstraction layer", which has slowly formed over the last decade or two.
Most of the work was done previously with the introduction of the various "AbstractX" classes, associated with texture cache cleanups and implementation of newer graphics APIs (Direct3D 12, Vulkan, Metal).
This PR simply takes the last step and yeeting these functions out of Renderer.
A new
AbstractGfxclass was created. It is now completely agnostic of any details from the flipper/hollywood GPU we are emulating, though somewhat specialised.In the future, I hope to make it possible to have two instances of AbstractGfx. One for the main video common, and one just for presenter to render the UI over the XFB and present it to the screen.
About the only thing that actually changed in this extraction was ClearScreen. Which has been replaced with ClearRegion and the ClearScreen logic was eventually moved into FramebufferManager (where a second version called ClearEFB already existed and mostly did the same thing)
Swap logic
This was a mess. Every time someone wanted something done once per frame, they would hack it into swap in slightly different ways.
It really doesn't help that dolphin didn't have a clean understanding of when a frame ended.
I've introduced a new event hooking system and created new
BeforeFrame,AfterFrame,BeforePresentandAfterPresentevents, with clear documentation about when they should be used.And everything that needs to do something once-per-frame now hooks those events directly.
Config Tracking
All the config state tracking has moved into VideoConfig.cpp. It also has a new ConfigChanged event, though I didn't wire that up everywhere.
EFB Scaling
This has been moved into FramebufferManager
Widescreen heuristic
This has been moved to a new
WidescreenManagerclass.Other changes
There are probably other things that I forgot. I've been working non-stop on this refactor for a week
Texture cache
The first three commits are from an older branch of mine, which got dragged along because I need them for async present.
TextureCache now uses smart pointers to handle TCacheEntry lifecycles. This simplifies the cleanup logic and allows for more complex ownership models (such as async present being able to hold onto it's XFB independently of texture cache)
I also added a content locking feature so that things outside of texture cache can lock the contents of the texture at a given point and prevent TextureCache from overwriting it. TextureCache will do a copy-on-write instead.
Known issues
input breaks when switching away from game windowpre-existing bug