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

Move Window and Gui calls to port game loop #787

Merged
merged 6 commits into from
Jan 11, 2025

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Jan 9, 2025

After some of the previous GUI/Window refactors, certain actions where no longer happening in the order that they originally were, leading to some issues happening like:

  • Game framebuffer calculation was a frame delayed and would use illegal values which caused garbage data to render in certain regions
  • Dropped frames on DirectX was still rendering the whole ImGui flow which would stall and hang certain GPU drivers leading to a crash

To address this, I've pulled out some of the GUI/Window handling out of Fast3D itself (gfx_pc) and created public calls that ports can call instead in the correct order to resolved the issues listed above.

Dropped frame support is pulled out of gfx_run() into a dedicated method Window::IsFrameReady() which ports can now use to bail out of all rendering.

Gui::Draw() was split into two methods Gui::StartDraw() and Gui::EndDraw() so that gfx_run() can be called by ports in-between the Gui calls, ensuring that the initial window is drawn and the Fast3D knows how much space is left for the game to render to.

Removed a previously added temporary fix Gui::SetupRendererFrame() now that the Draw calls have been split up.

@Archez
Copy link
Contributor Author

Archez commented Jan 9, 2025

I haven't removed Gui::Draw() yet, but as it is currently it is not compatible with the Fast3D render process so we should do something about it.

My thinking is there are three options:

  1. Remove it entirely and all ports replicate what the SoH PR is doing.
  2. Update and move Gui::Draw() to Window:Draw() and to take in a lambda which port side can pass in gfx_run(), this way gfx_run is called in the middle of the other draw calls correctly, without Gui needing to know anything about it.
  3. Create a public method on Window like Window::RunGraphicCommands() and move Gui::Draw() to Fast3DWindow::Draw() which would resemble what the SoH PR is doing currently.

I think option 3 is the most ideal, but making a public method means that all future Window variants would inherit the same function syntax of gfx_run(Gfx* commands, const std::unordered_map<Mtx*, MtxF>& mtx_replacements). This may or may not be desirable as the current vector for Mtx* on Fast3D side is very specific to the matrix interpolation system.

Therefore option 2 is probably more flexible. Online suggest lambda passed via std::function can introduce slight performance drops (due to type erasure and dynamic dispatch, but I have no idea how impactful that actually is). Or Lambda via a function template which does not incur a performance drop, but increases compile time due to templated headers. But I don't think templates can be used with our Window inheritance pattern using virtual class methods (compile time vs runtime).

Granted we don't know yet what other Window/renderers would look like today, so if anything we can always just change things up when that day happens.

@Archez Archez force-pushed the move-window-calls-to-port branch from ce69dc6 to d8d235c Compare January 10, 2025 22:24
@Archez
Copy link
Contributor Author

Archez commented Jan 10, 2025

This has been updated to go with option 3 outlined above, but instead of making the draw method a virtual on the base Window class, instead a dedicated method for Fast3dWindow::DrawAndRunGraphicsCommands() was made instead, which ports can cast to use.

The SoH PR has been updated accordingly.

This means LUS provides a sensible default draw call that ports can use with out having to worry about logic, but the underlying methods are all public in case a port wants to override the draw process.

@Kenix3 Kenix3 merged commit 7fc7376 into Kenix3:main Jan 11, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants