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

Introduce render context #1766

Merged
merged 6 commits into from Jan 27, 2023
Merged

Conversation

ZehMatt
Copy link
Contributor

@ZehMatt ZehMatt commented Jan 24, 2023

Closes #1762

@ZehMatt
Copy link
Contributor Author

ZehMatt commented Jan 24, 2023

I will have to get this a bit cleaner, instead of passing around the render target it should now pass the render context which holds an active render target, that will eliminate all the temporary getRenderContext calls.

@ZehMatt ZehMatt force-pushed the render-context branch 3 times, most recently from b6ae09c to 9fe4f64 Compare January 25, 2023 01:34
@ZehMatt ZehMatt marked this pull request as ready for review January 25, 2023 02:37
@ZehMatt
Copy link
Contributor Author

ZehMatt commented Jan 25, 2023

Unfortunately I can't refactor the RenderTarget argument just yet, there are some external functions that are missing and there might be shared use of the global render target instance. This has to do as a first pass until the rest is solved.

}
} // Impl

void SoftwareDrawingContext::clear(Gfx::RenderTarget& rt, uint32_t fill)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the reasoning behind this indirection? Why not just implement clear here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because at the moment everything passes RenderTarget but the context does not have its own state currently, I tried to do that but ran into a road block, the plan is to do eliminate that once we can do that.

Copy link
Contributor

@duncanspumpkin duncanspumpkin left a comment

Choose a reason for hiding this comment

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

All seems sensible

@duncanspumpkin
Copy link
Contributor

@AaronVanGeffen are you want a check as well.

Copy link
Member

@AaronVanGeffen AaronVanGeffen left a comment

Choose a reason for hiding this comment

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

Other than the commented-out header code that was pointed out, I have no objections 👍

@AaronVanGeffen AaronVanGeffen added this to the v23.01+ milestone Jan 27, 2023
@duncanspumpkin duncanspumpkin merged commit a6f603c into OpenLoco:master Jan 27, 2023
@ZehMatt ZehMatt deleted the render-context branch January 27, 2023 16:33
AaronVanGeffen added a commit to AaronVanGeffen/OpenLoco that referenced this pull request Apr 10, 2024
The internal progress bars weren't displaying at all due to recent changes in the graphics stack (OpenLoco#1766).
The progress bars use Gfx::renderAndUpdate to refresh the screen, but after this refactor, only
SoftwareDrawingEngine::render was being called, but the SoftwareDrawingEngine::present it introduced was not.
This left the screen not updated.
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.

Refactor software specific drawing functions from Gfx into SoftwareDrawingEngine
3 participants