-
Notifications
You must be signed in to change notification settings - Fork 34
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 cascaded shadow maps #1338
Add cascaded shadow maps #1338
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (2/2)
1923564
to
fd4adb1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/1)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1338 +/- ##
==========================================
- Coverage 40.73% 40.40% -0.33%
==========================================
Files 430 432 +2
Lines 30500 30745 +245
==========================================
Hits 12424 12424
- Misses 18076 18321 +245 ☔ View full report in Codecov by Sentry. |
5992420
to
52f8e80
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for working on this! 🎉 It got quite complex indeed, but I think you made a good job on making it as readable as possible 😌
Mostly pointed out areas of the code which could be slightly refactored to become a bit less complex. I still haven't been able to test this on my machine, but it would be nice if at least all of the reviewers assigned to this PR tried running it!
engine/include/cubos/engine/render/shadows/directional_caster.hpp
Outdated
Show resolved
Hide resolved
@mkuritsu will only be able to review this PR in 3 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the comments still to be resolved, LGTM!
4bd089b
to
e573c96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang-Tidy
found issue(s) with the introduced code (1/1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the change I mention LGTM!
9c9464b
to
aee5a20
Compare
aee5a20
to
e4710fc
Compare
e4710fc
to
4c783c2
Compare
…e2DArray glTexStorage3D doesn't exist in OpenGL 3.3
4c783c2
to
190159f
Compare
Description
Adds cascaded shadow maps, to allow for directional lights to cast shadows.
Checklist