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

Fixed handling of depth texture so it's resolved and bound when needed #27317

Merged
merged 1 commit into from Apr 1, 2019

Conversation

Projects
None yet
6 participants
@aqnuep
Copy link
Contributor

commented Mar 22, 2019

  • Cleaned up and improved the code determining when we need to use a depth
    prepass (previously it wasn't executed in certain cases even if it was
    needed)
  • Added code to prepare and bind the depth texture even when no depth prepass
    or MRTs (more precisely effect buffers) are used

Fixes #25870, fixes #25535, and fixes #25387.

@aqnuep aqnuep requested a review from reduz as a code owner Mar 22, 2019

@aqnuep

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Btw, a similar cleanup of the code for screen texture should probably be done as well, I just didn't want to creep too many changes (potentially prone to regressions) in one PR. The code is self-explanatory so anybody should be able to pick it up and do a similar change for screen texture handling.

@aqnuep

This comment has been minimized.

Copy link
Contributor Author

commented Mar 22, 2019

Also, this might be a good candidate to be part of the next 3.1 patch, as the bugs are present in 3.1 stable too.

@Chaosus Chaosus added this to the 3.2 milestone Mar 22, 2019

@bruvzg

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Checked it on macOS, unlike previous attempt to fix depth textures this PR doesn't cause any issues with 2x MSAA (#26258).

Fixed handling of depth texture so it's resolved and bound when needed
- Cleaned up and improved the code determining when we need to use a depth
  prepass (previously it wasn't executed in certain cases even if it was
  needed)
- Added code to prepare and bind the depth texture even when no depth prepass
  or MRTs (more precisely effect buffers) are used

Fixes #25870, #25535, and #25387.

@aqnuep aqnuep force-pushed the aqnuep:depth_texture_fix branch from cf439c9 to 849596c Mar 26, 2019

@QbieShay

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Cherry picked, seems to work with my project.

@akien-mga akien-mga merged commit 658aaa5 into godotengine:master Apr 1, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@akien-mga

This comment has been minimized.

Copy link
Member

commented Apr 1, 2019

Thanks!

If more users with 3D GLES3 projects could test this PR / the master branch on their projects, it would be great, to be sure that it's safe to cherry-pick in 3.1. (The 3.1 and master branches are still relatively close so the tests is meaningful even if done on master. You can also cherry-pick to 3.1 of course to test what 3.1.1 would be like.)

@QbieShay

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

My cherry pick is in 3.1-stable. been using it for a few days and it seems okay.

@lupoDharkael

This comment has been minimized.

Copy link
Contributor

commented Apr 1, 2019

Tested in my project and it works as expected.

@akien-mga

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

Cherry-picked for 3.1.1.

@Mr-Slurpy Mr-Slurpy referenced this pull request Apr 3, 2019

Closed

Not working with Godot 3.1 #4

@aqnuep aqnuep deleted the aqnuep:depth_texture_fix branch Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.