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

Screen space reflection with hierarchical ray tracer #94

Open
wants to merge 50 commits into
base: master
Choose a base branch
from

Conversation

fall2019
Copy link
Member

@fall2019 fall2019 commented Mar 10, 2024

Also add SSR mask to choose object receiving reflection.

hizssr.mp4

@fall2019 fall2019 added the enhancement refactoring or optimization label Mar 10, 2024
@fall2019 fall2019 requested a review from MomoDeve March 10, 2024 18:04
@fall2019 fall2019 marked this pull request as ready for review March 10, 2024 18:04
@MomoDeve
Copy link
Member

MomoDeve commented Mar 14, 2024

Hi, thanks for PR!
Have not start in-depth review process yet, but lets do couple of things to simplify the process and speed-up the merge

  1. I see there is a bunch of refactoring in shaders going on, this can be moved in a separate PR and be quickly reviewed
  2. I see you introduce a constant to conditionally compile code for compute & fragment shaders. I think this can be done in a different way, create common_utils file, then compute_utils & fragment_utils and include common there. With this, compute shaders can include compute_utils, fragment - fragment_utils, and we do not need conditional computation at all

@fall2019
Copy link
Member Author

fall2019 commented Mar 14, 2024

Hi, thanks for PR! Have not start in-depth review process yet, but lets do couple of things to simplify the process and speed-up the merge

  1. I see there is a bunch of refactoring in shaders going on, this can be moved in a separate PR and be quickly reviewed
  2. I see you introduce a constant to conditionally compile code for compute & fragment shaders. I think this can be done in a different way, create common_utils file, then compute_utils & fragment_utils and include common there. With this, compute shaders can include compute_utils, fragment - fragment_utils, and we do not need conditional computation at all

👌I will do it.
Refactoring of shader compilation

@fall2019 fall2019 self-assigned this Mar 14, 2024
@MomoDeve
Copy link
Member

MomoDeve commented Mar 28, 2024

I made a glass bricks (remove roughness texture, roughness=0, metallness=1). When I set "receieve SSR" I got degradation from 5ms to 20ms. Thats not acceptable performance. It should behave normal on all scenes

image
image

@MomoDeve
Copy link
Member

MomoDeve commented Mar 28, 2024

these artefacts under objects also need to be fixed. There should be a depth check
image

@MomoDeve
Copy link
Member

MomoDeve commented Mar 28, 2024

image Cost is way less than 1ms under level=2. Higher level also can produce less noisy result. This method can be the base to do further advanced optimizations like ray reuse and prefiltered samples in the future.

you are measuring CPU performance with that! There is no GPU performance profiling in engine yet

@MomoDeve MomoDeve self-requested a review March 28, 2024 19:59
Copy link
Member

@MomoDeve MomoDeve left a comment

Choose a reason for hiding this comment

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

this cant be merged with poor performance

Profile shader performance on different scenes and materials!

Use third party app like nsight to measure actual gpu performance and optimize the shader

please include the measurements after fixes

@fall2019
Copy link
Member Author

image
image

@fall2019
Copy link
Member Author

fall2019 commented Mar 31, 2024

Improved performance. Packed all depth maps into one texture and tested with different scenes.
Fixed artefacts under objects.
Fixed formats and warnings.
Performance issue is resolved rn. It takes from 1ms to 2ms.
image

@fall2019 fall2019 requested a review from MomoDeve March 31, 2024 07:18
@fall2019
Copy link
Member Author

fall2019 commented Apr 6, 2024

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement refactoring or optimization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Rendering] Introducing hierarchical z-buffer for fast screen space ray tracing
2 participants