DRD Team

Pull request: https://github.com/coelckers/gzdoom/pull/112

Okay, here's the 'lightmath' branch as a PR. As it is a little big I'll try summarize what it is changing:

1. Adds three new settings in the menus. A light math setting for the dynamic lights, and two for configuring the SSAO pass.
2. Renames FShaderManager to FShaderCollection and adds a new FShaderManager class. The purpose of this is to allow two families of scene shaders: normal shaders that only output to a single color buffer, and gbuffer shaders that use multiple render targets (MRT). This is needed by the AO pass to get the fog information for each pixel into a second color buffer.
   A bonus of this change is that it will enable the post processing shaders to get a more natural home in the future instead of being members of FGLRenderer. I'll probably make a follow up PR at some point where I move all the post processing shaders to be members of FShaderManager. That would mean that any kind of shader would always come from the shader manager (like it did in the past).
3. Changes the dynamic light position in the storage buffers uploaded to the GPU to be in eye/view coordinates instead of world. This is to simplify the math a bit in the main.fp shader.
4. Moves the dynamic light position a bit upwards. This is needed for flasks/vials that have their current center beneath the floor, but I'm not sure if the way I did it is a good way to deal with it. Maybe the lights.pk3 just needs to be fixed. Suggestions here are very welcome as it feels like a bit of a hack to just move them up in the code where I'm doing it (in gl_GetLight).
5. The SSAO pass itself runs as part of FGLRenderer::DrawScene. It sets up the shaders for MRT output, then draws the opaque stuff, then applies SSAO on top, switches back to normal shaders, and draws the transparent and portals. For each portal with SSAO it does the same thing. For performance reasons I added a gl_ssao_portals setting that controls how many portals it will do it for and allowed the user to adjust it in the menus. The portals.wad is very good for testing the trade off here between frame rate and visuals.
6. In GLPortal::End it outputs 0 into the alpha channel using a slightly modified stencil.fp. That allows the SSAO shader to know which pixels it should ignore the depth information with and is the primary way it avoids ssao'ing the sky geometry. We can make the primary main.fp also output 0 in the alpha channel if we have some parts of the scene that we don't want to be "seen" by the AO pass.
7. I left a comment in the code for main.fp saying what needs to be remarked/changed to start using model normals, when those become available.

Regarding 3:

I see that this adds a CPU-side rotation to every single dynamic light that gets processed. This was one thing I very intentionally did not do because it adds processing time right where it should not. The engine is already CPU bound so moving a costly matrix multiplication to the CPU is simplifying matters in the wrong place, I think.

For now I only want the SSAO but this is far too hefty to separate. Can you take the dynamic stuff out, please? This part won't be done until proper normals are available and unless that is ready a change like the above may do more harm than good.

I've updated the original post with a new PR that only includes the SSAO part of the original lightmath branch. Took quite a bit of work figuring out how to use git rebase and Tortoise Git's UI for this, ugh. Probably would have been easier to just ditch the commit history. On the plus side I've learned that when doing feature branches against a remote repository like this it is much better to occasionally rebase up against the remote than using merge.

About the eye space vs world thing, the shader math can be modified to work in world coordinates. The only catch is that then the GPU has to do this for each fragment rather than once per light on the CPU side. In any case, all this is now parked in a pointlight feature branch of mine.

dpJudas wrote:I've updated the original post with a new PR that only includes the SSAO part of the original lightmath branch. Took quite a bit of work figuring out how to use git rebase and Tortoise Git's UI for this, ugh. Probably would have been easier to just ditch the commit history. On the plus side I've learned that when doing feature branches against a remote repository like this it is much better to occasionally rebase up against the remote than using merge.

I honestly just branch off of the latest again, cherry pick if needed for whichever's the most relevant and slap in the rest of the code if needed. Every time I've tried to rebase through countless experiements it has always gone sideways.

The cherry-pick approach worked fine for getting out the point light shader math (it was just the two first commits in the original), but for the SSAO stuff it got a bit complicated because the eye buffers and exposure pass feature branches of mine had been changing the same files.

What I did discover was that if I had just done a "git fetch gzdoom" followed by "git rebase gzdoom/master" instead of git merge, then I would never have had any merge commits in the history and any conflict fixes would have been merged into the original commits. There's probably a catch to this that I'll find next time I do a long lasting feature branch.

I think it works like this:

Rebase merges your code in with the current master (an "under" merge) and the actual "merge" simply merges the current master over your code. (an "over" merge)

I think it's going to generate the same conflicts, either way.

Don't quote me on this, though, that's just a wild guess.

Yeah, it's the conflicts I kept getting and sometimes it wouldn't even allow me to repair some of them.

But then again there was a stupid glitch with SourceTree that's been fixed so I'll keep it in mind when I next do it.

That's how I understand it too Eruanna. But the difference is that once you resolved the merge with the rebase then it will appear as if you had branched much later from the master branch. That means the merge commits won't appear in history because when you resolve the conflict the commit afterwards will only have your resolved version. So it is more for Graf's sake that it makes much more sense to use rebase as that simplifies the commit history. It even allows you to "squash" several of your commits into one if you notice typos and such.

Well that's pretty neat. Sounds like rebase is a cool tool to have to keep things nice and tidy.

Almost like a rip-out-your-changes, purify, put-em-all-back. Does several things that you used to have to do manually.

In this case I probably would just have done two revert-commits to get rid of the unwanted code.

The reason why I want it out?
For the simple reason that this feature is about to be redone, so any change to the implementation that may prove unnecessary in the end is not good.
Once I have real normals, they'll be in world space, not eye space, so by converting the light positions on the C++ side it not only adds CPU processing time but in the end requires a matrix operation anyway to match two vectors in different coordinate spaces. I'd rather leave everything in world space then.

I agree that committing this together with the SSAO was a bad idea - I should never have used the same branch for those two things. It just happened to mean I had to figure out how to use git more closely. Using revert-commit sounds like what I probably should have done - oh well, at least I now know what the rebase command does.

I never used rebase. It's the one thing where I always felt it has no good use aside from damaging a repository. I always used cherry picking when rewriting commit history.

It is a dangerous tool for sure. I can see some use for it in the situations where I'd otherwise recreate a branch completely for PRs, but it is certainly not a tool to be used without being aware of exactly what it is doing and how.

So, I just tested it, but there's one strange glitch I noticed.

I went into the outside area in E1M2 on the eastern end. And every time I walk towards one of the 90° corners where the AO is most visible there seems to be some strange pulse effect, it looks like a one pixel wide strip where the effect is not properly applied. Can you explain why this happens?

Oh well, that didn't go too well. Have a look at the attached picture.

The map in question is Frozen Time
That white snowy area is one single sector, yet it's riddled with artifacts. I've also seen some artifacting in other places of that map which use small sector detail. The bright white snow textures make this stuff really stand out.

Equally bad, the frame rate on that map has taken a nosedive. With the last commit I get 50 fps at the most critical spot, overlooking the crossbeam bridges, but with the SSAO stuff included, it drops to 30 fps, regardless whether SSAO is active or not.