gemrb icon indicating copy to clipboard operation
gemrb copied to clipboard

Migrate rendering to SDL Shader API

Open bradallred opened this issue 2 years ago • 16 comments

I put together a graphics rendering roadmap a while ago detailing how we can solve some of the ongoing issues/performance/complexity with rendering.

Since then SDL has been awarded a grant from Epic games to develop shader capabilities for SDL_Renderer. This is huge news for us.

This would truly enable us to fix all the issues I mentioned in the roadmap without having to force OpenGL on people. We wouldn't have any direct dependency on OpenGL for these benefits, nor would we rely on hacks to control the active shader.

bradallred avatar Nov 12 '21 21:11 bradallred

That sounds great and I guess it will take a while to get the SDL side sorted out. Maybe a thing for 2023, we'll see.

lynxlynxlynx avatar Nov 13 '21 09:11 lynxlynxlynx

Ryan has added more information to the SDL discussion board

it looks like it will be a new API entirely. It may not change my roadmap plans much in the end. Time will tell.

bradallred avatar Nov 13 '21 20:11 bradallred

Btw, I'm not sure how useful it is for anything (besides learning that they use hq4x for scaling), but EEs also ship with a bunch of shader source files.

lynxlynxlynx avatar Jan 12 '22 22:01 lynxlynxlynx

that could be potentially very useful. lots of our blending is known to be incorrect (magic missile off the top of my head) and shaders can provide clues at the very least.

do you have a link or something?

bradallred avatar Jan 12 '22 22:01 bradallred

It's plain GLSL files, so if you open the game with NearInfinity, it will all be nicely categorized.

lynxlynxlynx avatar Jan 12 '22 23:01 lynxlynxlynx

I noticed that an RPi 400, with specs that should outnumber old Pentium platforms, really suffers from framedrops in some situations, like when flying over a map.

Profiling shows that GLSLProgram::SetUniformValue consumes a significant amount of time, also since its called millions of times. Looking at the current way, the switching between two shaders within one logical draw pass, that requires resetting the uniforms all the times, leaves me at one idea: Certrain things should be grouped for drawing, like glyphs or tiles, so that there is no need to call SDL and the driver with that administrative overhead all the time. (*)

(*) I have no tools right now for deep insights by profiling the GPU on the RPi. It's just that I looked a bit deeper at Vulkan over the last year and it tells the user: really think of how you put your drawable elements together, and that is sometimes orthogonal to what OpenGL 2 allows the user to do.

MarcelHB avatar Feb 26 '22 13:02 MarcelHB

I doubt that setting a uniform variable is actually a bottleneck. likely its caused by the fact that we keep switching programs and the call to SetUniformValue is just the point where the driver has to synchronize that change. We dont need to do anything complicated to address that. SDL already does a decent job of batching, we just throw it out the door by forcing a context flush. All of this is solved by using a unified shader for all sprites. Unfortunately, that requires wrangling control of that shader from SDL by modifying the shader program it is using. Not terribly complex tho and much simpler than trying to do any sort of grouping on our end.

bradallred avatar Feb 28 '22 21:02 bradallred

I doubt that setting a uniform variable is actually a bottleneck. likely its caused by the fact that we keep switching programs and the call to SetUniformValue is just the point where the driver has to synchronize that change.

Very likely, it was just the smell that lead me to this enlightenment of seeing how this is done way below optimum, since some paths switch between sprites & stencil programs every single time.

All of this is solved by using a unified shader for all sprites. Unfortunately, that requires wrangling control of that shader from SDL by modifying the shader program it is using.

As far as there is no API in sight for that, is this something well practicable? What I want to question is the way to plug-in custom OpenGL like this with its constant need for not breaking what SDL does around that while serving the most-general case.

If we have a unified shader for everything by us in the meantime: that's probably half the way already, not sure r/n if there something practically barring us from doing so already. I was just thinking of paths that not constantly go through BeginCustomRendering and EndCustomRendering for every single item of the same kind on screen as well.

MarcelHB avatar Feb 28 '22 23:02 MarcelHB

As far as there is no API in sight for that, is this something well practicable?

this issue was opened because SDL is adding an API for that. But if you're talking about solution prior to that, then I believe there are OpenGL APIs to modify an existing shader program. We wont break anything in SDL by usurping its RGBA shader. Besides, our existing shader already mimics it for geometry purposes anyhow.

If we have a unified shader for everything by us in the meantime: that's probably half the way already, not sure r/n if there something practically barring us from doing so already.

everything is an SDL_Texture and we have no access to the underlying texture handle. Everything goes though SDL_RenderCopy, even our stenciling. It's really easier and cleaner to just take over the shader. which would indeed eliminate the need for BeginCustomRendering and EndCustomRendering as well as open other avenues of increasing performance such as hardware accelerated palette swapping.

bradallred avatar Mar 01 '22 00:03 bradallred

this issue was opened because SDL is adding an API for that.

I've tried to find a roadmap or updates but not sure if and when there is something to see, months, a year, or more?

But if you're talking about solution prior to that, then I believe there are OpenGL APIs to modify an existing shader program. We wont break anything in SDL by usurping its RGBA shader. Besides, our existing shader already mimics it for geometry purposes anyhow.

Hm, it's easy to retrieve the currently loaded program ID if we have a point in time where this state is safe to assume, i. e. SDL has completed its shader loading. But I've never tried rebuilding and resetting a program that was linked previously already. Worth a try in any case.

MarcelHB avatar Mar 01 '22 18:03 MarcelHB

I've tried to find a roadmap or updates but not sure if and when there is something to see, months, a year, or more?

Ryan has said he will be publishing a plan in the "coming months" (this was late last year), we wont know more until then.

Hm, it's easy to retrieve the currently loaded program ID if we have a point in time where this state is safe to assume, i. e. SDL has completed its shader loading.

my plan was to do this is to do this in SDL20VideoDriver::CreateSDLDisplay by drawing the scratchBuffer to the screen and presenting the renderer to force the shader we want to usurp into being active.

bradallred avatar Mar 01 '22 18:03 bradallred

I have played around a bit here, it's actually easy I just never had that use case before:

1. glGetIntegerv(GL_CURRENT_PROGRAM, &programID) where safe to use
2. glGetAttachedShaders(programID)
3. glDetachShader() twice
4. load, compile, attach GemRB shaders
5. re-link programID
6. also maybe check what uniforms are actually required to update (saves our internal lookups at minimum)

It gives us about 15% improvement with some cheap cuts (reducing # of SDL calls in the CustomRender functions). It does not pull the RPi performance out of the mud, but the profiler is cleaned up and shows the next significant CPU beggars (like fog, map exploration) clearly.

Dealing with SDL is easy, probably saves us a lot of calls and code, so looks the main part is then to merge the fragment shaders into a single program, so I vote for it.

MarcelHB avatar Mar 04 '22 16:03 MarcelHB

you should submit a PR :)

bradallred avatar Mar 04 '22 16:03 bradallred

Ok, I'll try that. Will probably just take a little more time to redo this cleanly and thoroughly tested.

MarcelHB avatar Mar 04 '22 16:03 MarcelHB

Technically, I found a solution for most of the things so far. Anyway, there is something interesting I'm stuck at for now. Look at:

shader

With fog, everything looks good except for the areas in range of sight, they are black (without fog, everything is). What's even more interesting is that this problem goes away if I leave that alive (at some arbitrary time before the SDL_RenderCopyEx):

https://github.com/gemrb/gemrb/blob/f20bd793fd78e2763d931313766a2047bb2fbf3a/gemrb/plugins/SDLVideo/SDL20Video.cpp#L248-L253

I don't really want to leave it there since this is part of the overall idea. But then there is something I haven't found in the SDL state mgmt or our map draw code. Will continue to look later.

MarcelHB avatar Mar 06 '22 18:03 MarcelHB

you should probably just open a WIP PR so I can get a better idea of what's happening. Also would be a better place to continue this conversation since it has nothing to do with the upcoming SDL shader API :)

bradallred avatar Mar 06 '22 22:03 bradallred

this work is about to land in SDL 3. We should find some time to review the API in case #1771 needs to be nudged in a particular direction.

bradallred avatar Jan 12 '23 17:01 bradallred