asc-community / MxEngine

C++ open source 3D game engine
BSD 3-Clause "New" or "Revised" License
1.12k stars 59 forks source link

Screen space reflection with hierarchical ray tracer #94

Open fall2019 opened 6 months ago

fall2019 commented 6 months ago

Also add SSR mask to choose object receiving reflection.

https://github.com/asc-community/MxEngine/assets/53587145/19f3379f-ca8f-493c-8308-5c1f32531eef

MomoDeve commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

Also added some comments to simplify the review process and reduce maintenance efforts.

fall2019 commented 6 months ago

Conflict resolved

fall2019 commented 5 months ago

Resolved Comments.

MomoDeve commented 5 months ago

Hi, sorry for delaying the review! Can you explain why you used intensity in CameraSSR? Its not a physical term. I suppose metalness of surface should be used instead?

fall2019 commented 5 months ago

Hi, sorry for delaying the review! Can you explain why you used intensity in CameraSSR? Its not a physical term. I suppose metalness of surface should be used instead?

Yeah.MetallicFactor is used in apply_ssr_fragment. That means you can set it to 1.0 to keep it physically accurate and set to other numbers when you want it more artistic.😁

fall2019 commented 5 months ago

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.

MomoDeve commented 5 months ago

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 commented 5 months ago

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

MomoDeve commented 5 months ago

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

fall2019 commented 5 months ago

image image

fall2019 commented 5 months ago

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 commented 5 months ago

image