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

feat(rendering): Add Shadow Normal Offset Bias in the deferred shading #1370

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

GalaxyCrush
Copy link
Contributor

Description

I added the Shadow Normal Offset Bias to the deferred shading and gave the user the option to use or not use this bias.

Checklist

  • Self-review changes.
  • Evaluate impact on the documentation.
  • Ensure test coverage.
  • Write new samples.
  • Add entry to the changelog's unreleased section.

@GalaxyCrush GalaxyCrush requested review from RiscadoA, tomas7770 and a team as code owners November 18, 2024 19:14
@GalaxyCrush GalaxyCrush linked an issue Nov 18, 2024 that may be closed by this pull request
@GalaxyCrush GalaxyCrush requested review from mkuritsu and RodrigoVintem and removed request for a team November 18, 2024 19:14
Copy link
Contributor

github-actions bot commented Nov 18, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/preview/pr-1370/
on branch gh-pages at 2024-11-22 19:00 UTC

Copy link
Contributor

@RodrigoVintem RodrigoVintem left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@RiscadoA RiscadoA left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! LGTM

engine/assets/render/deferred_shading.fs Outdated Show resolved Hide resolved
engine/assets/render/deferred_shading.fs Outdated Show resolved Hide resolved
engine/include/cubos/engine/render/shadows/caster.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@mkuritsu mkuritsu left a comment

Choose a reason for hiding this comment

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

LGTM! Just a small change in the variable types

engine/src/render/deferred_shading/plugin.cpp Outdated Show resolved Hide resolved
engine/src/render/deferred_shading/plugin.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@tomas7770 tomas7770 left a comment

Choose a reason for hiding this comment

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

LGTM! Just tested some of the demos from the cubos-demo repo and noticed significant improvements!

engine/include/cubos/engine/render/shadows/caster.hpp Outdated Show resolved Hide resolved
@tomas7770
Copy link
Contributor

Also, don't forget to update the changelog!

@GalaxyCrush GalaxyCrush force-pushed the 1308-frequent-shadow-acne-andor-peter-panning branch 2 times, most recently from 22b1f94 to 7341b8d Compare November 22, 2024 01:59
Copy link

codecov bot commented Nov 22, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 54.22%. Comparing base (e32490e) to head (09ba33f).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
engine/src/render/deferred_shading/plugin.cpp 0.00% 2 Missing ⚠️
engine/src/render/shadows/caster.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1370      +/-   ##
==========================================
- Coverage   54.23%   54.22%   -0.01%     
==========================================
  Files         438      438              
  Lines       25338    25341       +3     
  Branches     2347     2347              
==========================================
  Hits        13742    13742              
- Misses      11596    11599       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@GalaxyCrush GalaxyCrush force-pushed the 1308-frequent-shadow-acne-andor-peter-panning branch from 7341b8d to 09ba33f Compare November 22, 2024 18:58
@GalaxyCrush GalaxyCrush merged commit 0a8fd95 into main Nov 22, 2024
11 checks passed
@GalaxyCrush GalaxyCrush deleted the 1308-frequent-shadow-acne-andor-peter-panning branch November 22, 2024 19:40
@RiscadoA RiscadoA added this to the 0.5 milestone Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Frequent shadow acne and/or peter panning
5 participants