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

[NO SQUASH] Enforce batching all render requests in SDL2_Renderer #3139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swagtoy
Copy link
Contributor

@swagtoy swagtoy commented Dec 11, 2024

For all SDL2 Render requests, we (presumably) may not be batching it. This is noticed in the SDL Painter for functions like SDLPainter::draw_texture as well as other classes. This means that for our RenderCopyEx calls (technically, SDL_RenderCopyExF) as well as any shape functions, SDL was immediately passing each RenderCopy call to the GPU. Note: I believe this could be the case, though the documentation is somewhat unreliable in SDL's defense on it. It calls one case "efficient" and the other "compatible"...

With this PR, by settings a single SDL2 Renderer hint, this should result in a possible major performance boost!

They also mention on the docs that it is disabled by default due to some potential issues with usage of rendering, but after some pretty general testing, I did not spot any in particular. I would appreciate some testers that want to boot a build up and see if it at least draws stuff.

SDL3 mentioned in a PR that this is default now, but since we use SDL2 still, let's just pass this.

@tobbi
Copy link
Member

tobbi commented Dec 11, 2024

You missed out on a huge opportunity: You could've called the branch: batch-please

@swagtoy
Copy link
Contributor Author

swagtoy commented Dec 11, 2024

@tobbi Ha, my new favorite running joke is from our player class, m_wants_buttjump, so the branch name is oriented off of that :)

@tobbi
Copy link
Member

tobbi commented Dec 11, 2024

Honestly, I couldn't find any differences in terms of speed between this branch and latest master using SDL.

@swagtoy
Copy link
Contributor Author

swagtoy commented Dec 11, 2024

I don't think it'll actually be noticable really, obvious i wouldnt notice it either simply because my computer is just fast. Its just a small task to ensure batching gets done?

Copy link
Member

@MatusGuy MatusGuy left a comment

Choose a reason for hiding this comment

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

I think an actual practical review would be more helpful so I'm not gonna approve this

@swagtoy
Copy link
Contributor Author

swagtoy commented Dec 16, 2024

Sure, just don't forget that you were the one that told me about this :^)

I can dig into the SDL2 code and see if changing the flag actually does anything on my machine, for example. Then we can decide further. It may be a redundant flag and we just don't know it.

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