Skip to content
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

Add configurable shadow rendering #124

Merged
merged 12 commits into from
Feb 6, 2020
Merged

Add configurable shadow rendering #124

merged 12 commits into from
Feb 6, 2020

Conversation

yanchith
Copy link
Member

@yanchith yanchith commented Jan 29, 2020

The renderer can now draw shadows via shadow mapping. This is used to collect
shadows from rendered meshes and project them onto the ground plane (which is a
plane mesh primitive).

The CommandBuffer::draw_meshes_* family of functions now has a flag, whether
the drawing should cast shadows. The DrawMeshMode enum has a new
FlatWithShadows variant that can collect shadows. The structure is much more
flexible internally in the renderer, but not yet exposed. Also, the renderer now internally supports transparency, but the only way to render a transparent object is to provide a transparent color to the flat_shading_color option - that color is used for DrawMeshMode::FlatWithshadows.

Known problems:

  • Vulkan validation errors for image layout transitions (this is probably not
    our fault, but it looks scary),
  • Screenshots with transparent background can now see ground, because
    it is not transparent. We can alternatively not render the ground plane for
    screenshots.

@yanchith yanchith requested review from ondrowan and janper January 29, 2020 16:08
This is just a draft. Don't merge, only evaluate.

The renderer can now draw shadows via shadow mapping. This is used to collect
shadows from rendered meshes and project them onto the ground plane (which is a
plane mesh primitive).

The `CommandBuffer::draw_meshes_*` family of functions now has a flag, whether
the drawing should cast shadows. The `DrawMeshMode` enum has a new
`FlatWithShadows` variant that can collect shadows. The structure is much more
flexible internally in the renderer, but since the renderer does not support
transparency yet, the outside usage is restricted.

Known problems:

- Vulkan validation errors for image layout transitions (this is probably not
  our fault, but it looks scary),
- Screenshots with transparent background can now see ground, because
  it is not transparent. We can alternatively not render the ground plane for
  screenshots,
- The ground is not transparent, objects are not visible from below,
- We needed to disable the ground and shadows for `Edges` shading mode, because
  edges don't write their depth, meaning the ground would overwrite them
- Possibly much more...

Please evaluate this PR and let me know if we want to merge it as-is (or with
minor fixes), not merge it at all, or work on the transparency issues.
@yanchith yanchith force-pushed the viewport-ground-shadows branch from 046cfaa to 4044b98 Compare January 29, 2020 16:13
janper
janper previously approved these changes Jan 29, 2020
Copy link
Collaborator

@janper janper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice and it does all that we discussed.
Let's disable shadows for transparent screenshots.

@yanchith
Copy link
Member Author

Thanks! I'll have a fresh look at the code later and clean it up before merging.

@yanchith
Copy link
Member Author

yanchith commented Feb 2, 2020

I've given it some thought and think that proper (or at least not terribly incorrect) transparency is within our grasp. Probably less than a day of work. I'll update the PR with that next week.

If anyone wants to review the code, I can merge this first and add transparency with another PR to make the reviewer's job easier.

@yanchith
Copy link
Member Author

yanchith commented Feb 4, 2020

Ready for review.

@yanchith yanchith marked this pull request as ready for review February 4, 2020 23:34
@janper
Copy link
Collaborator

janper commented Feb 5, 2020

Nice. A couple of notes and questions:

  1. There seems to be this glitch when looking from below It was happening also before the shadows, so let's ignore it. FYI, the sphere is set to 32x32 segments. glitch
  2. You mentioned the shadows can't be more blurry. Is it still the case and why?
  3. Once we have a transparency, it would be nice to have some very transparent endless ground because it's confusing when there is no visible shadow because it's inside of the model. If truly endless is a problem, it can be a disk/plane that gradually fades out into full transparency.
    image
    Untitled2

@yanchith
Copy link
Member Author

yanchith commented Feb 5, 2020

There seems to be this glitch when looking from below It was happening also before the shadows, so let's ignore it. FYI, the sphere is set to 32x32 segments.

Yeah, I believe that is a matcap rendering glitch, I've seen it before too. Don't exactly know what's causing it tbh.

You mentioned the shadows can't be more blurry. Is it still the case and why?

Yes. For the same reason as before. We are already smoothing the shadows by doing PCF (percentage closer filtering) https://developer.nvidia.com/gpugems/gpugems/part-ii-lighting-and-shadows/chapter-11-shadow-map-antialiasing

We could increase the number of samples to get marginally better results, but my machines already struggle with it when running on intel GPUs - this would make us require capable hardware for everyone, unless we had a flag to turn it down. Not sure if that's worth it.

I can show you how to up the shadow quality so that you can experiment with different values.

Currently, we treat the ground plane as just another mesh in the renderer that just uses a different ShadingMode, one that can catch shadows. We could treat it specially and implement some kind of postprocess based shadow blurring just for the ground plane, but that sounds like both a lot of work and also will have performance implications.

Once we have a transparency, it would be nice to have some very transparent endless ground because it's confusing when there is no visible shadow because it's inside of the model. If truly endless is a problem, it can be a disk/plane that gradually fades out into full transparency.

Not sure what you mean. We already have a fully transparent (not endless, but very large) ground. If the objects were transparent, you would see the shadow there, and you already can in the Edges rendering mode. Are you saying that you don't want it to be fully transparent? We can already make it not fully transparent and it behaves as on your pictures around the models, but then the edges of the ground are visible. To solve that, we could blend it into full transparency on the edges using a texture mask (or some clever shader trick). I'd keep that for a separate PR though.

@janper janper self-requested a review February 5, 2020 16:45
janper
janper previously approved these changes Feb 5, 2020
Copy link
Collaborator

@janper janper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as it is what I saw at your computer, we are good to merge. Have you considered setting the ground plane size according to scene's bounding box though?

@yanchith
Copy link
Member Author

yanchith commented Feb 5, 2020

Have you considered setting the ground plane size according to scene's bounding box though?

Ok, I'll do it

@yanchith
Copy link
Member Author

yanchith commented Feb 5, 2020

I still discovered some screenshot transparency issues with the new transparent ground. I'll need to investigate. Will merge after that is fixed.

@yanchith yanchith merged commit 46366a3 into master Feb 6, 2020
@yanchith yanchith deleted the viewport-ground-shadows branch February 6, 2020 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants