-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
base: master
Are you sure you want to change the base?
Conversation
You missed out on a huge opportunity: You could've called the branch: |
@tobbi Ha, my new favorite running joke is from our player class, |
Honestly, I couldn't find any differences in terms of speed between this branch and latest master using SDL. |
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? |
There was a problem hiding this 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
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. |
A quick glimpse at the SDL2 branch @ https://github.com/libsdl-org/SDL shows that a SDL3 indeed removes this flag entirely, but we are on SDL2 so until we decide to migrate, enabling this flag cannot hurt for the SDL2 side. It may not be a major speedup actually, but it should be a helpful optimization for the SDL2 Renderer. The android side on SDL2 shows that this doesn't happen however, just letting you know. |
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 ourRenderCopyEx
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.