Percentage-Closer Soft Shadows #1107
Replies: 41 comments 91 replies
-
|
Thanks Chris for posting this work. Once the implementation is further along I'll merge with VSG and vsgExamples master, as a step in this direction I have merged the changes as a VSG separate-shadow-samplers branch and a vsgExamples soft-shadows branch: https://github.com/vsg-dev/VulkanSceneGraph/tree/separate-shadow-samplers I've done a first past review of the changes to the VSG, it looks like we should be able to use the new createDescriptorImage convenience functions. I want to test the changes against the original cascaded shadow map implementation, if that works fine then I'll get these changes to the VSG merged with master, hopefully wrapped up today. Looking at vsgExample changes to shaders suggest that the phong shader is the most mature and as there is an overlap between the phong and pbr shaders w.r.t shadows I think it's probably time we created a dedicate shadow.frag/shadow.glsl file that is included by phong or pbr shaders to reduce the amount of duplication. I would also like look at controlling whether Percentage-Closer Soft Shadows is enabled in the shaders and the settings used. Once I have spent more time with the new shader and C++ code I have a better idea of what changes to vsg::ViewDependentState and vsg::Light might be required. One change I have been considering is moving the vsg::Light::shadowMaps value out of vsg::Light and leaving this to vsg::ViewDependentState. Adding extra light size property to vsg::Light to help guide the size of penumbras seems like something we might consider as well. |
Beta Was this translation helpful? Give feedback.
-
|
I have been testing the soft-shadows branch of vsgExamples and found that the modified standard_phong.frag that defaults to num of samples to 8 I get obvious under sampling issues, but upping this to 16 I get significantly better results and obviously going all the way up to 64 produces very nice results. To harder to judge performance impact without creating a set of test models and animation paths. From my initial tests with just the simple vsgshadow test model I'm seeing 2750fps with 8 samples, 2131fps for 32 samples. I'm thinking that ViewDependentState should pass in the number of samples to use. The angleSubtended is something I would put into vsg::DirectionalLight and pass to the shader via the LightData. Defaulting to sun and earth makes sense. In the shader I see that the tan(angleSubtended/2) is used so perhaps ViewDependentState can compute this. The use of the inverse of the shadow map matrix is something that could be done by ViewDependentState. |
Beta Was this translation helpful? Give feedback.
-
I used the convenience functions to create the sampler descriptors, but the sampled image descriptors needed The prior shadow implementation should work just fine with the separate sampler and image provided it's adapted to combine them in the shader. That should just mean changing layout(set = VIEW_DESCRIPTOR_SET, binding = 2) uniform sampler2DArrayShadow shadowMaps;
...
texture(shadowMaps, vec4(sm_tc.st, shadowMapIndex, sm_tc.z)).rto layout(set = VIEW_DESCRIPTOR_SET, binding = 2) uniform texture2DArray shadowMaps;
layout(set = VIEW_DESCRIPTOR_SET, binding = 4) uniform sampler shadowMapShadowSampler;
...
texture(sampler2DArrayShadow(shadowMaps, shadowMapShadowSampler), vec4(sm_tc.st, shadowMapIndex, sm_tc.z)).rAs an aside, this made me realise I'd forgotten to put some of the shadow implementation in the PBR shader, so I've just pushed another commit that copies and pastes the rest.
Some of the difference will be the aforementioned stuff I forgot to copy and paste until just now. I definitely agree that splitting it our into a separate reusable file is a good idea. This also makes switching implementations easier - even if multiple implementations end up in the same file, it prevents filling all the lighting shaders with several shadow implementations, but there's also the option of putting each shadow implementation in its own file.
Eight was essentially chosen arbitrarily, but with the sampling pattern I ended up on, the undersampling artefacts are a lot less bad than they were with much higher sample counts and all the other ones I tried first. I'm definitely in favour of it being parameterised, but I'm not sure
The precomputable part of the trigonometry's currently left in the shader because while its inputs are The inverse light space matrix is in the shader because I wanted to gauge your appetite for using up more of the light data buffer on something like that before making things more complicated. |
Beta Was this translation helpful? Give feedback.
-
|
As per your suggestion I have updated the vsgExample standard_phong.cpp and stadard_pbr.cpp shaders and updated the built-in ShaderSet in the separate-shadow-samplers branch. With this changes vsgshadow in vsgExamples master works fine. It does look like we have a redundant sampler being added that is only required for the soft shadows, but as I expect soft shadows to be merged with VSG master within the next week I think this is fine: I have created a PR for these changes: If the automated build tests go smoothly I'll merged with VSG master. which will allow folks to test out the new PCSS shaders in vsgExamples soft-shadows branch with VSG master. |
Beta Was this translation helpful? Give feedback.
-
|
#1109 is now merged with VSG master so no need to pull in any VSG branches to work with @AnyOldName3 work on Percentage-Close Soft Shadow shaders in the vsgExamples soft-shadows branch. For the very latest updates to PCCS implementation you might want to keep track of Chris' forks. |
Beta Was this translation helpful? Give feedback.
-
|
Neither of us has changed these bits to reflect the new layout yet. I assume that's something we need to do, right? https://github.com/vsg-dev/vsgExamples/blob/master/examples/utils/vsgshaderset/phong.cpp#L60 |
Beta Was this translation helpful? Give feedback.
-
|
Good catch, the ShaderSet's need to have the new descriptors added. |
Beta Was this translation helpful? Give feedback.
-
|
Something that I've alluded to on one of our calls, but I've only just confirmed is a real problem is the depth clamping. If all you care about is whether something occludes light, it makes sense to enable depth clamping for the shadow map RTT and restrict light space to only cover the frustum as it means there's more depth precision available where it matters. However, when PCSS is used, you also care about how far away occluders are, and depth clamping squashes a bunch of potential occluders onto the shadow camera's near plane. This means that any occluders outside the view frustum potentially end up with much narrower penumbras than they're supposed to have, and this can look particularly bad when zooming in etc. as they grow and shrink before your eyes. The solution would be to disable depth clamping and extend light space to encompass all potential occluders rather than just all potential receivers. Currently, disabling depth clamping in the vsgShadow example only makes the first change, which just means that a bunch of occluders are left out and some shadows are totally missing, or even worse, totally missing from just one shadow map, causing an abrupt cutoff. I'm under the impression we don't necessarily know the bounds of the set of potential occluders for a shadow map, so would need an arbitrary large amount of depth precision to be wasted, but at the worst, a bound could be set based on the blocker search radius and angle subtended by the light. With the current default of 1m, we'd 'only' need 220m of shadow map depth range to be nearer the near plane than the nearest shadow receiver. As a side note, while calculating that I realised I'd got the angle subtended by the Sun on Earth wrong by a factor of ten. When I use the correct value, the soft shadows are a lot less impressive in the example scenes than the screenshots above. Reducing the penumbras from 36cm to 3.6cm obviously has a big impact, not least because lots of shadow map texels aren't much smaller, so end up visible. On the plus side, if a new planet gets discovered much, much closer to the sun than Mercury, we know that what I've created would work pretty well for simulating the shadows there. |
Beta Was this translation helpful? Give feedback.
-
|
W.r.t depth clamp, currently this is enable for the whole scene graph, my plan is to specialize the shadow map rendering so only depth clamp is used for it. This doesn't change the issue you raise but as a heads up that I may do some work in this area. |
Beta Was this translation helpful? Give feedback.
-
|
W.r.t blocker search requiring unclamped shadow/depth values, we don't know the extents of that the shadow map requires unless we run a compute bounds traversal, something that is expensive so not something we'd want to do on the fly. So we could pick a conservative value for the near plane of the shadow map, pushing it back towards the light to make sure it captures all the geometry that we think is relevant. I think you are suggesting that do this but limit how far we push the shadow map near plane back. When originally implementing the shadow map rendering I did experiment with not clipping the depth value with the intention of just having unbounded floating values rendered to the shadow map. I didn't get this to work, but I may well have just been doing it wrong. If it's possible then it might be worth seeing if this is possible rather than resorting to using the depth clamp. |
Beta Was this translation helpful? Give feedback.
-
|
I thought of an alternative that might work on a wider range of hardware. If as well as the depth buffer, an R32F colour buffer was bound, then the unclamped depth could be written as the colour, and then the colour buffer could be used as the shadow map. |
Beta Was this translation helpful? Give feedback.
-
|
Would you need a depth buffer at all if you are using that workaround? Would the shadow map samplers be fine with use of a colour buffer? Seems like a thing to try to see what happens, |
Beta Was this translation helpful? Give feedback.
-
Yes, as the depth test still needs to happen. Otherwise, you couldn't guarantee that the occluder in a particular texel was the closest one to the light, it'd just be whichever was drawn last.
Now we're using separate samplers, it's just a single-component float texture. The only thing that would be different would be changing the layout from |
Beta Was this translation helpful? Give feedback.
-
|
Double the effective depth buffer memory will also double the memory bandwidth on write so will be slower to generate the shadow maps. Given we can look at look at disable depth clamp or shifting the near plane back for the shadow maps I think these two approaches have more promise for fixing issues without adding overhead so these are the avenues I think we should look at first. |
Beta Was this translation helpful? Give feedback.
-
|
As if the shadow maps nearest the eye that currently has the smallest depth range and causing issues with clamping and too small occluder distance then perhaps pushing back the near plan and the shadow maps but a constant amount might be sufficient for avoiding the worst of the potential artefacts. |
Beta Was this translation helpful? Give feedback.
-
|
@robertosfield Does vsg-dev/vsgExamples@31f288f seem like the right approach? |
Beta Was this translation helpful? Give feedback.
-
|
I'm not able to do a proper review till I back to work on Monday but on quick review it looks sensible. |
Beta Was this translation helpful? Give feedback.
-
|
I've got a performance comparison from yesterday's change with made it so the inverse shadow matrix is computed on the CPU instead of in the fragment shader: 64 samples
8 samples
So I think it's fair to say it barely helps, if at all. I don't have an explanation for why it ends up slower for the single-light single-shadow-map 64-sample case, but it's consistent when I come back to it, so doesn't appear to be down to something like a background task running on my machine while I was doing that one test. |
Beta Was this translation helpful? Give feedback.
-
|
I tried redoing this without the cache coherency issue outweighing the effect, and then got a kernel-mode driver crash when doing the last run, which meant I lost all my results. I can't be bothered doing it again right now, so I'll just note down my analysis:
|
Beta Was this translation helpful? Give feedback.
-
|
I've learned something about the performance variation. Maybe twenty minutes ago or so, I was reliably getting 700 FPS when running a particular test, and now I'm reliably getting 900 FPS running the exact same thing. I have all the same background applications running, and as far as I can tell, they're all doing consistent things and leaving plenty of CPU cores idle and the GPU unused, so shouldn't be adding much noise to the results. I think this is a strong sign that the unexpected performance regression I was concerned about yesterday isn't real, but it does call into question all the benchmark results I've got that weren't back-to-back (and potentially some of the ones that were, too). I'll need to investigate further to see if I can isolate a cause. |
Beta Was this translation helpful? Give feedback.
-
|
Well I've found a trigger. If my music player is visible on my second monitor, it costs me about 20% of my framerate. If it's entirely covered by other windows, or is minimised, then I get that 20% back. I don't think a static picture of some album art and a progress bar should be so expensive, so I'm going to message AMD about it. |
Beta Was this translation helpful? Give feedback.
-
|
Redone without that interference:
So I'd still conclude that the effect is less than the amount of noise in the results, and that I'm still not sure it's worth using up the light buffer space for the extra matrix. |
Beta Was this translation helpful? Give feedback.
-
|
Some more investigation about the impact of the stochastic rotation of the poisson disk. Obviously, the most obvious impact of this is that it's going to convert artefacts with patterns into noise, which should be less visually impactful. When there are only a few samples, the most obvious problem is fairly severe banding, and that gets pretty effectively turned into much less severe noise. However, it would make intuitive sense if the banding wasn't a problem in the first place when the sample count was turned up high, but it's still clear that there's some noise under certain conditions when you look closely at the shadows even with sixty-four samples, so there must be some kind of signal that it's scrambling to create that noise. Disabling the rotation makes its causes pretty clear:
From a little local testing, I've determined that most of the noise visible through the rotation is caused by 64 samples not being enough to totally eliminate banding, unless the resolutions really bad. That's irritating, as it's not like simply throwing more samples at the problem is an entirely viable solution. The results from the previous post demonstrate that going from eight samples to sixty-four halves the framerate, so going from 64 to 128 might well halve it again, but will only halve the intensity of the noise. However, I don't think this justifies abandoning the current approach and switching to moment shadow maps or anything like that. I imagine that a big part of why I'm noticing this is that I've spent a long time staring at these shadows so am hyper-aware of any defect that most people wouldn't notice. Additionally, the noise is very attenuated by a gentle blur or small amount of scaling of the final image (in fact, you might not be able to see there's noise at all in GitHub's preview until you click the image to see it at its original resolution), so I'd expect that any application using shadows in addition to bloom, depth of field, or temporal anti-aliasing won't have visible noise. Also, most applications will be using textured meshes instead of solid colours like this test scene, and the noise will be far less significant than colour variations from textures. 8 samples, no rotation
8 samples, rotation
64 samples, no rotation
64 samples, rotation
|
Beta Was this translation helpful? Give feedback.
-
|
Also, I just did a quick test of the various techniques that now exist on my branch.
Eight samples (for soft techniques)
|
| technique | average framerate |
|---|---|
--pcss |
557.26 |
--pcf |
646.871 |
--hard |
711.961 |
--none |
708.328 |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for all the details. It sounds like you've close to exhausting what you can do with this type of PCSS algorithm. Do you feel the implementation is close to ready to merge with the main VSG? It's a curious finding that the CPU inverse is not noticeably faster, this suggests that read memory bandwidth is a significant cost vs cost of computing an inverse. This doesn't sound right but your observations are hard to explain away otherwise. Trying different hardware might be informative. What hardware are you working on for these tests? Have you tried looking at the results with multisampling enabled? |
Beta Was this translation helpful? Give feedback.
-
|
These tests are on a Radeon 7800 XT on Windows at 1440p. I know that in OpenGL on modern AMD hardware, UBOs don't end up in the same fast memory as plain uniforms, but there's a large cache that's equally fast, so if that cache is shared with textures, it's plausible that the shadow map sampling fills the cache with shadow map data, evicting the light data, but on the other hand, it's also plausible that the shadow map sampling just leaves loads of free cycles to calculate the inverse in that would otherwise just be spent waiting for reads to finish, or even just that the matrix inverse just ends up being such a small fraction of what the shader does that removing it only has a very small performance impact. It's kind of inherent with this style of PCSS that you'll end up with lots of texture reads being conditional on the result of other texture reads, and that's never going to produce particularly nice machine code for a GPU. There might not be much more that can be done to improve it without doing something drastic like blocking off a potential future implementation of perspective shadow maps (in which case, some matrix transforms can be turned into simple scaling for parts of the maths, and bits of the shader might become easier to rearrange) or we switch to passing the frustum split distances to the shader and then picking the shadow map to use based on the eye-space Z coordinate rather than whether the light-space position ends up within bounds for a particular shadow map (which makes it easier to write more GPU-friendly code). On the other hand, there are some things I could play with now I've got a consistent setup for comparing performance, and I might discover a nice surprise of some kind. |
Beta Was this translation helpful? Give feedback.
-
|
Another bit of data:
The result for 0.01 units with eight samples is particularly interesting as it's faster than fixed-radius PCF at 0.05 units, despite doing extra maths and twice as many texture accesses. Together, these numbers are a strong sign that the biggest problem is simply waiting to sample bits of the texture that aren't in the cache, and there might be decent gains to be had by reordering the sample points in the poisson-like disk so bits of the sampled texture near to each other get sampled in quick succession, reducing the chance relevant cache lines have been evicted. The tool I used to generate the sampling disk as a feature intended for this that I haven't used yet, so I'll see if I can generate something that runs faster without sacrificing the progressive property that allows the same disk to be used for small and large sample counts. |
Beta Was this translation helpful? Give feedback.
-
|
It looks like the tool either does the cache optimisation or maintains the progressive property, but can't do both (and also the cache optimisation seems a little naive as it's just sorting points by their y coordinate). However, it definitely impacted performance:
|
Beta Was this translation helpful? Give feedback.
-
|
I can probably give it a try, though. It shouldn't be hard to come up with a basic proof of concept and get a rough estimate of how good it could be. |
Beta Was this translation helpful? Give feedback.
-
|
For performance I would have thought cache coherency of the shadow map read is the most critical part, with the sampler disk of less importance so only reading every second/third/fourth/fifth point would have less impact. |
Beta Was this translation helpful? Give feedback.
-
|
Yes, I agree with that, but the fact that you're mentioning it makes me think I didn't get my point across clearly. We get cache coherency for the shadow map read by ensuring each point we sample is reasonably close to other ones we've sampled recently, which means each point in the sampling disk is spatially close to its predecessor. This still needs to hold when we only use a subset of the disk, but there are plenty of ways to do that, e.g. if every point is close to the one before and after it, then the one before it will also be fairly close to the one after it. However, we also need the sampling disk to be a good sampling disk that gives us a good representation of the signal it's sampling. Most of the subsets you can take of a good sampling disk aren't themselves good sampling disks. The existing sampling disk was generated with a tool that would produce a set of sampling points that could produce a decent sampling disk with a subset of any size of its points, and ordered them such that you'd be able to get it by using the first n points. We can't have the sampling points sorted both of these ways at the same time as they're kind of opposites - with the first sort, point What I'm proposing is putting what are currently the first four points at indices 0, 16, 32 and 48, then the next four points at indices 8, 24, 40 and 56, then the next eight at indices 4, 12, 20, 28, 36, 44, 52 and 60, and so on. This will then mean you can access the four points in the optimal four-sample disk by accessing points at |
Beta Was this translation helpful? Give feedback.
-
|
Results for an interleaved cache-optimised still-progressive poisson-like sampling disk, when the cache optimisation is just a y sort (I tried a cleverer approach, but 32! is too big):
So that's nearly all the benefit with only a minor change to how things work. I'm happy to call this the least bad option, and by a significant margin. |
Beta Was this translation helpful? Give feedback.
-
|
The changes are now pushed. |
Beta Was this translation helpful? Give feedback.
-
|
I tried merging your vsgExamples soft-shadows branch with main repo's vsgExamples soft-shadows branch but I didn't get any shadows exact for when using --technique hard, using pccs I don't see anything shadow. WIth soft I do see shadows but not if I use more than one shadow map. When I use the --technique soft I only see to get shadows for the what is probably the nearest shadow map. I tried use your repos soft-shadows branch directly as well but get the same result. Could there a check-in missing? |
Beta Was this translation helpful? Give feedback.
-
|
Have you got this commit for the VSG? master...AnyOldName3:VulkanSceneGraph:inverse-shadow-matrix Without it, the inverse shadow matrix won't be included in the light data, so only |
Beta Was this translation helpful? Give feedback.
-
|
I haven't tried that change to the VSG, I will try it out. How much more needs to be done to wrap up the soft shadows work? Once we merge the work I'll bump the VSG version to reflect the addition. |
Beta Was this translation helpful? Give feedback.
-
|
I have merged the change to the VSG as soft-shadows branch to match the branch in vsgExamples. I'll merge changes to vsgExamples next. |
Beta Was this translation helpful? Give feedback.
-
|
I have merged the soft shadows work into VSG and vsgExamples as soft-shadows branches: https://github.com/vsg-dev/VulkanSceneGraph/tree/soft-shadows Let me know if I've made any mistakes in the merge. To test things I tried: vsgshadow --technique hard --sm 3 --dc --direction 1 1 -1.0
vsgshadow --technique pcf --sm 3 --dc --direction 1 1 -1.0
vsgshadow --technique pcss --sm 3 --dc --direction 1 1 -1.0In testing I found that all of the techniques are hitting up clipping of the shadow map, even with depth clamp on. Unfortunately Github doesn't allow me to attach .vsgt files so I've renamed them to .txt: problem_small.txt To test it out for the large dataset I use this command line: vsgshadow --technique hard --sm 3 --dc --samples 8 --shadow-samples 64 --direction 1 1 -0.5 -n 2 --large --sd 1000 -p problem_large.vsgtI tested this same path and command line with VSG + vsgExamples master and see the same problem so this clearly isn't soft shadows specific issue. It seems to be low angle light direction that seems to provoke it. |
Beta Was this translation helpful? Give feedback.
-
|
This seems to suppress the problem: diff --git a/data/shaders/shadows_hard.glsl b/data/shaders/shadows_hard.glsl
index 55767b4..f71d1b6 100644
--- a/data/shaders/shadows_hard.glsl
+++ b/data/shaders/shadows_hard.glsl
@@ -19,18 +19,30 @@ float calculateShadowCoverageForDirectionalLight(inout int lightDataIndex, inout
vec4 sm_tc = sm_matrix * vec4(eyePos, 1.0);
- if (sm_tc.x >= 0.0 && sm_tc.x <= 1.0 && sm_tc.y >= 0.0 && sm_tc.y <= 1.0 && sm_tc.z >= 0.0)
+ if (sm_tc.x >= 0.0 && sm_tc.x <= 1.0 && sm_tc.y >= 0.0 && sm_tc.y <= 1.0 && sm_tc.z >= 0.0 && sm_tc.z <= 1.0)
{
matched = true;
overallCoverage = texture(sampler2DArrayShadow(shadowMaps, shadowMapShadowSampler), vec4(sm_tc.st, shadowMapIndex, sm_tc.z)).r;This condition makes a difference when the fragment is closer to the light than the shadow RTT's near plane, and it's pretty obvious that potential casters in that region will be clipped if depth clamping is disabled, or clamped to be further from the light if it's enabled, so there's no reason to think that this was ever a good idea. In OSG-based OpenMW's shadow implementation, I made it skip the opposite condition, i.e. when the fragment is further from the light source than the shadow RTT's far plane, as there'd be plenty of potential occluders, and they'd have higher resolution than they would in the next shadow map. However, even then, I still had to make it use the next shadow map, too, otherwise occluders beyond that shadow map's far plane would be ignored. We could get the same effect here by swapping the existing |
Beta Was this translation helpful? Give feedback.
-
|
As another simplification step I have modified the vsgshadow example so that it sets up the shader hints just once then uses this for the phong and pbr ShaderSets: This change is checked into vsgExamples soft-shadows-simplfied branch. There is still more stuff I'd like to get things ready for merging with master, but will be to return to this tomorrow. |
Beta Was this translation helpful? Give feedback.
-
|
I have refactored the way vsgshadow sets up the ShadowSettings and shaderHints so it's all done more locally: I'm now wondering about how we might combine the defines required into the ShadowSettings subclasses. |
Beta Was this translation helpful? Give feedback.
-
|
I checked in an shader optimization to the soft-shadows-simplfied branch of vsgExamples that changes how the loops are testing and exited: With the huge_medieval_battle_scene test model:
The PCF cases is a bit skewed because it's using a penumbra radius of 0.1 on model that is so small, I've also changed the example to enable the radius to be passed in I get 64fps, I've left the same values as the original test though so we can see like for like. A structuring of the PCSS shader a bit further so the that occluder search use the same approach I've done for the final sampling might net additional improvement in fps. As part of the changes I've made today I've changed vsgshadow so that it takes --hard, --pcss and --pcf radius command line options for toggling the technique rather than the previous --technique command line option. This allows vsgshadow to be a bit more centralized and easier to follow. I'm seeing a regression vs VSG/vsgExample master in vsganimation with hard shadows but I haven't yet got to the bottom of this. |
Beta Was this translation helpful? Give feedback.
-
|
I have reservations about the correctness of this optimisation. It no longer skips over unused light data and shadow maps, so shadow casting for the next light will use the wrong data. This seems to be the core of the change, and I don't see a way to keep the performance benefit without leaving support for multiple lights broken, as the only change to control flow is that it skips past all the things that get the indices ready for the next light. |
Beta Was this translation helpful? Give feedback.
-
|
The handling is just done differently, the accounting for skipping unused shadow maps should all be there. The main skip is now standard_pbr.frag and standard_phong.frag. I tested the multiple line multiple shadow case when I original made these changes, and just running it again shows it's working fine. vsgshadow -n 3 --sm 3
vsgshadow -n 3 --sm 3 --pcss
vsgshadow -n 3 --sm 3 --soft 0.1All work correctly as far as I can tell. The code is simpler and faster than it was before. Please read all the changes to the shaders. |
Beta Was this translation helpful? Give feedback.
-
|
I've found it now - there are other changes that aren't in the commit you linked that make it work, with vsg-dev/vsgExamples@fbd0655 being the important one. I'm actually pretty surprised 1b03783 made any difference to anything as by the time it was made, all it did was avoid writing to some variables that wouldn't be read from again, which glslang should be clever enough to eliminate, and the operations involved were just multiplications and additions, which shouldn't have made enough difference to show above the noise floor. Are the results definitely a before and after for the commit you linked? |
Beta Was this translation helpful? Give feedback.
-
|
Please just test out VSG master and vsgExamples master head as I have made many commits over the past 4-5 days, some fix regressions. For further refinements I would suggest branching VSG.vsgExample master and working from there. Tomorrow I'll looking at a RegionOfInterest functionality for the scene graph/shadow techniques. My aim with this will be controlling the shadow map extents with a user definable scene graph mechanism. This is orthogonal to soft shadow maps so will be a seperate branch of work. Hopefully I'll wrap it up tomorrow. |
Beta Was this translation helpful? Give feedback.
-
|
I've investigated a bit further, and think I can clarify a bit:
Hopefully that brings us back onto the same page. |
Beta Was this translation helpful? Give feedback.
-
|
I have add the ability to override the ShadowSettings per Light, or as a catch all in ViewDependentState: Changes to VSG: vsg-dev/vsgExamples@888070c This change should make it possible to have different View's with different ShadowSettings such as reducing/increasing visual quality. |
Beta Was this translation helpful? Give feedback.
-
|
@AnyOldName3 I'm now close to being ready to merge the soft-shadows-simplified branch with VSG master. Are there issue you can think of that I should wait for? I am thinking about renaming the shadow_pcf.glsl and associated VSG_SHADOW_PCF to shadow_soft.glsl and VSG_SHADOW_SOFT respectively as PCF is a bit cryptic. We could add doxygen and shader comments explaining the algorithm details. |
Beta Was this translation helpful? Give feedback.
-
|
I have decided to go ahead ad renamed VSG_SHADOW_PCF to VSG_SHADOW_SOFT as this should be more intuitive to users: |
Beta Was this translation helpful? Give feedback.
-
|
During testing I found the vsgtextureprojection example was no longer creation textures so I've updated the vsgExamples/data/shaders/textureprojection_phong.frag to be consistent with the new standard_phong.frag that utilizes the new shadow*.glsl shaders: The following change also had to be made to enable shadows: auto shaderHints = shaderSet->defaultShaderHints = vsg::ShaderCompileSettings::create();
if (numShadowMapsPerLight>0)
{
shaderHints->defines.insert("VSG_SHADOWS_HARD");
}Previously hard shadows were supported out of the box without the need for any additional defines, so the soft-shadows branch work will break them as things currently stand. We could possibly just have VSG_SHADOWS_HARD always be built into the shaders. This would at least help existing application mostly work as before when they update to VulkanSceneGraph-1.1.3. I am thinking about adding a ShadowSettings std::string define member variable that could be used to pass on to the ShaderSet configuration. The HardShadow::define would default to VSG_SHADOWS_HARD, SoftShadow::define to VSG_SHADOWS_SOFT and PercentageCloserSoftShadows::define to VSG_SHADOWS_PCSS. ShaderSet's don't know about ShadowSettings so figuring out which optional code paths to enable would still need to be done explicitly by applications even if it could be made less hardwired than passing a specific "VSG_SHADOWS_*" string. |
Beta Was this translation helpful? Give feedback.
-
|
I have decided to return the phong and pbr ShaderSets to enable VSG_SHADOWS_HARD code path by default so compatibility with older versions of the VSG is a bit more seamless: Users will still need to change light->shadowMaps = numShadowMaps;to light->shadowSettings = vsg::HardShadows::create(numShadowMaps);But this necessary to provide a coherent and extensible means of defining the desired shadow rendering technique so key part of evolution of the VSG's API to handle increasingly sophisticated types of rendering out of the box. I have now tackled all the issues I've spotted in today's review and testing so I'm ready to merge with VSG/vsgExamples master. I'll do one more round of review & testing then merge. |
Beta Was this translation helpful? Give feedback.
-
|
I have now merged the soft-shadows-simplified branches of the VSG and vsgExamples with the respective masters: |
Beta Was this translation helpful? Give feedback.
-
|
@AnyOldName3 I'm replying here as trying to navigate nested replies is painfully out of chronically order. W.r.t optimizations, we are working on different OS, hardware and of course drivers and test models so I'd expect some variations. The performance regression with storage buffer usage is troubling. For an variable sized data structure a storage buffer is better fit, but if it's hammering performance then we may need to think about making it fixed sized. If it's cache optimization issue that your hardware/driver is hitting up against then then storing the inverse matrix may part of the problem. |
Beta Was this translation helpful? Give feedback.
-
|
In OpenGL, the spec said that uniform buffers had to end up in the same fast memory as GL2-era uniforms, whereas storage buffers could be backed by slower VRAM (and didn't benefit from all the same caches as textures do). Not all hardware followed the letter of the spec, e.g. OpenMW has a separate path for Intel integrated GPUs that uses array uniforms instead of UBOs as that's faster on that hardware, but slower everywhere else, and I've read that AMD GCN GPUs treated UBOs and SSBOs as the same thing but relied on a largeish cache to make smaller buffers fast for read-only access. However, I'm on a newer Navi card, which, like I believe post-Maxwell Nvidia cards do, is supposed to have dedicated memory on-chip for UBOs. I'm not entirely sure this is actually the case, as the reported maximum SSBO and UBO size under Vulkan is the same large value, which wouldn't make much sense if there was a small dedicated piece of fast memory, whereas looking at https://vulkan.gpuinfo.org/listreports.php, I see contemporary Nvidia GPUs list a comparatively tiny value for the maximum UBO size while the SSBO size is still big. It could just be the case that AMD's driver just uses the UBO/SSBO distinction as a hint for the caching strategy, and might end up putting small SSBOs in the 'dedicated fast UBO' memory and leaving big UBOs out of it as and when it feels like doing so. |
Beta Was this translation helpful? Give feedback.
-
|
Anyway, regardless of what I'm seeing, the difference should be bigger on your machine, as it's got an Nvidia GPU, which definitely has dedicated UBO memory. I'm assuming you measured before and after the switch to storage buffers and didn't see a difference? I guess maybe Nvidia's driver could be implicitly converting small read-only SSBOs into UBOs instead. |
Beta Was this translation helpful? Give feedback.
-
|
It might be worth taking vsg-dev/vsgExamples@master...AnyOldName3:vsgExamples:speedy-bodge for a spin on hardware other than mine before I put effort into doing it properly. It seems to give me maybe a 10% boost, but I cut corners (e.g. hardcoding things) and don't know how much of the benefit will stay once everything's done right. |
Beta Was this translation helpful? Give feedback.
-
|
Some timing data for that commit (when reapplied to the StorageVsUniform branch):
The main idea behind the change was that instead of iterating over the shadow maps in the outer loop, and the sample points in the inner loop, it'd iterate over the sample points in the outer loop, and over the shadow maps in the inner loop, which then let the expensive texture sampling operation get moved out of the inner loop so divergence could be reduced. Because it would be flitting between the shadow maps, I also made it so the shadow matrices were kept in a local array, otherwise it'd just exacerbate the SSBO problem. This is a bit of a problem, as that array needs as many elements as a light might have shadow maps, and that's not something that's bounded, so I just hardcoded it to three for now. It might turn out that the intermediate array isn't necessary, or turns out not to be necessary if the shadow matrices are provided in a certain way (e.g. in a |
Beta Was this translation helpful? Give feedback.
-
|
I didn't do before and after performance test from the storage buffer change. I made this change for both jointMatrices used for skinning and the lightData. Both I changed to avoid the hardwiring of sizes in the shaders. I will need to create a branch to test uniform vs buffer so it can be tested on a range of system. I have an Intel LInux lapto, and Intel Windows desktop both with integrated graphics, a AMD 5700G Linux destop but it has my Geforce 2060 plugged in. I will need to formularize a set of performance tests. |
Beta Was this translation helpful? Give feedback.
-
|
I have created StorageVsUniform branches of VSG and vsgExamples to experiment with performance of storage and uniform buffers for animation joints and light data: https://github.com/vsg-dev/VulkanSceneGraph/tree/StorageVsUniform Currently only vsganimation has code to enable use of uniform rather than storage buffer, simply add --ubo ti the command line i.e vsganimation ~/Data/glTF-Sample-Assets/Models/BrainStem/glTF/BrainStem.gltf
vsganimation ~/Data/glTF-Sample-Assets/Models/BrainStem/glTF/BrainStem.gltf --uboI will update vsgshadow to have similar codepath for enabling uniform buffer. |
Beta Was this translation helpful? Give feedback.
-
|
I have updated the StorageVsUniform branches of VSG and vsgExamples to enable testing of storage vs uniform buffers for the lightData. This allows us to do tests with storage buffer (default) or uniform buffer (enabled with --ubo) such as: vsgshadow --large -p saved_animation.vsgt -t --duration 5.0 --sm 3 --pcss
vsgshadow --large -p saved_animation.vsgt -t --duration 5.0 --sm 3 --pcss --uboThe animation path I used is attached but renamed to have .txt extension, this just needs to be removed to use it as above: Results on my AMD 5700G + Geforce 2060 linux system shows uniform buffer is faster by 22% for hard shadows, 12% for PCSS and 6% for Soft shadows (with a penumbra of 1.0 for the above test.) I did see a noticeable difference in vsganimation with a skinned model so it looks like use of storage buffer in the vertex shader has far less impact than use in the fragment shader so it may be that we can split the ticket. I will now test on my Intel Linux laptop and my Intel Windows 11 desktop both with integrated graphics. |
Beta Was this translation helpful? Give feedback.
-
|
I have now run the same tests on my Intel Core i7-13700 desktop system and hard shows is around 7% faster with uniform buffer than storage buffer, but for both Soft/PCF and PCSS I see no differences, some runs one is faster than the other and visa-versa. I now need to move my Geforce 2060 into my Intel desktop to see if the OS and proprietary drivers make a difference, and whether the AMD5700G integrated GPU shows any difference. |
Beta Was this translation helpful? Give feedback.
-
|
On my machine, now I've got that branch to make testing things easier, I'm seeing no difference in framerate with SSBOs vs UBOs unless I take measures to ensure there's lots of cache pressure. If I only have one light, or have the PCF/PCSS blocker search radius only as big as a few shadow texels, or only take a few samples of the shadow map, the performance is identical. Once I introduce cache pressure (three lights, 10m PCF radius, 64 shadow samples), the difference grows to 18%. That supports the idea that on Navi-era AMD hardware, SSBOs compete for cache occupancy against textures, whereas UBOs are cached somewhere else. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the testing and insights Chris. I have moved my Geforce 2060 into my Windows 11 desktop and run with the latest NVidia drivers. Curiously my hard shadows frame rate is 85% higher under Linux than under Windows - 912fps vs 493fps. Even more curious the hit with soft and pcss is much lower under Windows than Linux and may be performing better. Unfortunately for these tests I only noted the % between storage and uniform buffers rather than absolute frame rates so can't provide numbers for this observation. Unfortunately moving a card between two machines is a bit of pain so I'll not attempt to get to the specifics today. With Windows11/2060 & NVidia x driver I am seeing a 2%, 6% and 11% better performance with using uniform buffers rather than storage buffers. Last datapoint I can gather today is the AMD 5700G integrated GPU on my Linux desktop system, once I have this I will attempt to draw some conclusions and come up with a short and longer plans for how to get the overall best performance configuration out of the box. |
Beta Was this translation helpful? Give feedback.
-
|
I have now tested my Linux desktop system with the AMD5700G's integrated GPU, for all the tests I get identical results between uniform buffer and storage buffer. So from all our tests it looks like we see between 0% and 22% improvement with using uniform buffer rather than storage buffer, so my change to use storage buffer for both joint matrices and light data caused this performance regression on some hardware/driver combinations. Short term I think changing back to uniform buffer for the light data makes sense, but leave the joint matrices as storage buffer. I would like to make the ability to toggle the descriptor types easier for the ViewDependentState types, the StorageVsUniform branches kinda do this but in a quick and dirty hacky way. Given that it's performance regression, not a an actual bug perhaps it'd be fine to just leave with the current storage buffer until I come up with a cleaner way. Perhaps a ViewDependentState could "have a" set of vsg::DescriptorBindings that could default to what the standard built-in ShaderSet's use for ViewDependentState, but allow uses to override them if they use their own ShaderSet with it's own DescriptorBindings, This would also touch upon the ViewDependentStateBinding/CustomDescriptorSetBinding that the ShaderSet's use to integrate ViewDependentState but as this has always felt a bit hacky perhaps there is some coherent solution hidden away that I just need to spend a few hours teasing out. Not a task for today though as I've already been waylaid from my original plans getting to the bottom of this performance regression. I don't think it is worthwhile trying to performance optimize the shadow functionality till I resolve what to do about the above and to change the default behavior back to use uniform buffers. Oh... one of the issues with uniforms buffers is that you have to set a fixed size in the shaders for the lightData array, something that storage buffer avoids. The fixed size is really a hack to get things to work reasonably well out of the box, it's not an ideal solution. I did try using specialization constants to set the lightData[] arrays size but GLSL/SPIR-V didn't allow this. So... we're left with using something like #prama(tic) shader composition to inject the size to use, but... the VSG doesn't yet support setting a value for the #defines so that's possibly another feature I need to implement to address this problem. |
Beta Was this translation helpful? Give feedback.
-
One of the examples for specialization constants in the GL_KHR_vulkan_glsl spec is setting an array size, so I'm surprised to hear that, unless it's a bug in glslang which might have been fixed. I just tried layout(constant_id = 17) const int arraySize = 12;
layout(set = 7, binding = 0) uniform TestUniform
{
mat4 matrices[arraySize];
} mats;and it at least compiles. |
Beta Was this translation helpful? Give feedback.
-
|
On Thu, 4 Apr 2024 at 16:52, Chris Djali ***@***.***> wrote:
I did try using specialization constants to set the lightData[] arrays
size but GLSL/SPIR-V didn't allow this.
One of the examples for specialization constants in the GL_KHR_vulkan_glsl
spec is setting an array size, so I'm surprised to hear that, unless it's a
bug in glslang which might have been fixed. I just tried
layout(constant_id = 17) const int arraySize = 12;layout(set = 7, binding = 0) uniform TestUniform
{
mat4 matrices[arraySize];
} mats;
and it at least compiles.
It could have been a bug/limitation in an older version of glslang. It's
something I tried last year and just couldn't get it to work.
|
Beta Was this translation helpful? Give feedback.
-
|
@AnyOldName3 This morning I made a series of changes to the VSG to help optimize rendering performance, merging changes to use Uniform Buffer for LightData and then introduction of VSG_ALPHA_TEST #define into the build-in ShaderSets. Performance tests I've done showed the value of these changes, so the defaults will now perform better. I have also created a branch to test whether computing the inverse shadow map matrix on the CPU and passing to the GPU in the LightData uniform buffer or computing the inverse shadow map matrix on the GPU when required was better. The branches that tests this out are:
Running vsgshadow with a new --smi command line option moves the compute of the inverse shadow matrix into the PCSS shader and out of the LightData. Performance test of hard, soft and PCSS shadows on my Linux desktop AMD5700G, Linux Intel i7 Laptop, and Windos 11 Intel i7+Gefore2060 desktop failed to show any performance differences between the two configurations except for a 5.7% difference on my AMD5700G where --smi (computing in PCSS shader) was slower than what is in VSG master/computing the shadow matrix on the CPU. I am surprised that there was no measurable penalty for having the extra LightData usage for the hard and soft shadow code paths as neither use the inverse shadow matrix. I am also surprised there there is no measurable cost in computing the inverse shadow matrix on the Intel i7 onboard GPU on my laptop or the Geforce 2060. I like the simplicity of doing the inverse when required on the GPU, but making PCSS a little ~6% on an AMD5700G and similar hardware is not an easy trade. So for now I think I'll now merge changes to VSG master to move the inverse computation back into the PCSS shader. Perhaps this is something to revisit in the future. |
Beta Was this translation helpful? Give feedback.
-
|
@AnyOldName3 I have changed the fragment shaders to use the suggest specialization constant approach for setting the lightData[] size: #1169 These changes are now part of VSG and vsgExamples master. |
Beta Was this translation helpful? Give feedback.
-
|
It's probably a good idea to document the range of specialisation constant IDs that the VSG reserves. They don't have to be consecutive, and there are a lot of different |
Beta Was this translation helpful? Give feedback.
-
|
I'll put it on my TODO list... kinda long though so might take me a while to clear off the critical stuff and get to low priority stuff like this. |
Beta Was this translation helpful? Give feedback.
-
I've done some work to implement Percentage-Closer Soft Shadows into the VulkanSceneGraph. It's available on this branch of vsgExamples and requires this branch of the VSG. So far, I've not updated the precompiled shaders as the VSG branch isn't technically dependent on the vsgExamples one, so to see them in action, you'll need to use options that force the shaders to be compiled from source at runtime, or precompile them yourself.
Here are a few screenshots:
Basic scene with five blocker samples and eight shadow samples
This angle actually has several cascade boundaries, but they're well-hidden, which can be a problem when combining PCSS with cascaded shadow maps.
Basic scene with lots of samples
Large scene
Nearly everything's just been changes in the shader - the only change to the VSG itself is having
ViewDependentDatapass the shadow map array and samplers as separate uniforms instead of using a combined image sampler, which allows the blocker search to access depth values, but the shadow samples to still benefit from hardware PCF.The basic approach (after a decent amount of prototyping - there's a description of each decision in the commit history) is:
I've kept the current VSG approach of selecting shadow cascades based on whether a sample point lies within that cascade's light-space bounds. This has made things more complicated than some implementations, which would instead pass the view-space depth range for each cascade to the shader and select based on that, but avoids ignoring a better-quality shadow map in the regions where it spills beyond its intended boundary. There'll be some performance impact of this, although I can't quantify it, but I didn't want to override decisions that had already been made just to make this a bit simpler - it can always be changed later.
Blocker search
The blocker search is conducted across each cascade before any shadow sampling is done - if another approach is taken, there's a risk of missing potential blockers beyond the bounds of a given shadow map, and calculating an inappropriate penumbra radius because of that. The sample points use the same rotated poisson-like disk as the actual shadow sampling, but with a fixed radius - I intend to turn this into a specialisation constant, define or uniform as the most sensible value depends on the scene, its scale and the number of sample points used. If the blocker search radius is too large, and sample points too sparse, it becomes likely that the most important occluders directly between the centre of the light and the shadowed object are missed, and holes appear in the shadow. There are ways to calculate the maximum necessary blocker search radius, but typically it's large and would require many, many blocker samples to ensure nothing important was missed, so most of the time, knowing the nature of the scene, a more appropriate value can be selected manually.
At the moment, the blocker distances are averaged in light-space, but the average blocker for a particular shadow map is converted back to eye space. The first reason for this is that it allows the distances from different cascades with different variants of light space to be combined.
Penumbra radius
The second reason the average blocker is converted to eye space is that not all versions of light space preserve angles between lines. That means that if a technique like Light-Space Perspective Shadow Maps, or even worse plain Perspective Shadow Maps (neither of which I recommend as one of the maniacs who's previously implemented one), are used, we can't know the angle subtended by the light source in light space. We need to know that angle to know how big the penumbra from the average blocker would be. Currently, it's another constant set based on how big the Sun appears on Earth, but I'll eventually move it to the directional light data as not everything people want to render is on Earth and non-fictional. The actual radius calculation is just super-simple trigonometry.
Shadow sampling
As mentioned already, sampling is done using a rotated poisson-like disk scaled to the penumbra radius. Being poisson-like, it avoids moiré patterns and similar problems caused by having sample positions be too ordered, but unlike an actual poisson disk, it has the additional properties that:
The literature I've historically found about shadow map sampling didn't make it clear how much a difference a good poisson-like sampling disk would make over a bad poisson disk, so I thought this was interesting to discuss here.
The rotation for the poisson disk is calculated based on the same maths as Godot (also MIT-licenced) uses - it takes a hash of the pixel position using a formula originally from Crytek. Other techniques I tried led to obvious banding, significant moving noise patterns or noise that changed so much from frame to frame that surfaces looked like an untuned analogue CRT screen. I didn't keep screenshots, but the Godot approach just worked far better than anything else, especially with low sample counts.
Like with the blocker search, each shadow map in turn is tried with all the sample points until the total amount that landed within valid light space reaches the target threshold.
The result of having both the blocker search and shadow sampling work this way is really consistent penumbras across cascade boundaries - I can't find them even in screenshots where I know where they should be.
Beta Was this translation helpful? Give feedback.
All reactions