Shadow Overhaul#7529
Conversation
|
Nice job, I like what I'm seeing for the most part. But I don't know if I like having two separate model drawing pipelines. It seems like a potential point of divergence if modifications get added to the color model drawing functions but isn't reflected in the shadow pass or vice versa. Seems like a potential maintenance hazard. Virtually all engines I worked on share the same model drawing path with color and shadows so I don't see why we need to do it here. Everything else seems good. One recommendation is to randomly rotate the Poisson disc per fragment to get even better PCSS results. |
|
The reason I did this was cause the old combined pipeline was a stateful mess, with 90% of the main pipeline not relevant to shadows. Also, after all, the only thing they really share deep down is the submodel iteration, and transform handling. |
|
Yeah if I were to redo the model queue code now, we'd just queue all the models once then reuse the draw list for different passes. You can sort of see glimpses of that with regards to the opaque vs transparency passes. Ideally we would rely on one pipeline and queue draws into shadows, opaque, then transparency all at once with each add_buffer_draw(). But yeah I get why you did what you did though. At the very least perhaps the add_model_draw method could shared by the regular model draw and shadow model draw? Conceptually the regular model draw is basically the new shadow add_model_draw just with extra crap. Maybe we can sweep all the extra legacy crap into a function or two and that could be the delta? But I get it if you don't find that worth doing? Maybe once I'm done stabilizing the Vulkan PR I can try to take a stab at what I described earlier in this post. |
|
That would amount to merging I feel like, the only sensible way to do it is as you describe.
I could try to merge the child-walking functions and maaaaaybe the parent render_model_queue, but I'll need to think if I can find a good way to make sure that neither model_material nor model_render_params touch the shadow side of the code, since that's just mostly invalid for shadows. However, I feel like it's not worth it, for the precise reason that if you do decide to redo the rendering as you describe, that'd make this all obsolete anyways. And if you don't, I probably will do that once I'm through my current ToDo list, though for sure in a different PR, as that needs its own dedicated testing and would be a long PR in itself that we don't just want to tag onto this one. |
fbaff5c to
f6e65d8
Compare
# Conflicts: # code/def_files/data/effects/main-g.sdr
f6e65d8 to
00abd57
Compare
This refactors how FSO handles shadows pretty fundamentally.
First, the user-facing changes:
Then the performance enhancements (with reference to the points in #7499):
This results in an overall good speedup, as such, closes #7499. Note that on my machine, shadows are rarely the bottleneck cause my GPU is fairly overkill, especially in the physics-heavy scenes where the framerate drops low. Benchmark is Icarus:

I've had informal tests from @wookieejedi on FotG (which benefits doubly due to the second large buffer clear for the cockpit shadow pass being optimized out entirely), with reports of the framerate effectively doubling from low 30's to consistently over 60.
Backend-wise, this PR also has some cleanup and fixes.