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

[no sq] Fix ECF_D32 support in ogles2 video driver #15396

Merged
merged 2 commits into from
Nov 8, 2024

Conversation

grorp
Copy link
Member

@grorp grorp commented Nov 7, 2024

fixes #15395

OES_depth32 only talks about support for render buffers, not textures, so it's not relevant here: https://github.com/KhronosGroup/OpenGL-Registry/blob/main/extensions/OES/OES_depth32.txt

This fixes the scene being black with "video_driver = ogles2" and "enable_post_processing = true" on my desktop computer.

To do

This PR is a Ready for Review.

How to test

ogles2 on Android: things should work
ogles2 on desktop: things should work

@grorp grorp added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Bugfix 🐛 PRs that fix a bug @ Client rendering labels Nov 7, 2024
@grorp grorp changed the title Ogles2 fix [no sq] Don't claim support for ECF_D32 format with ogles2 driver Nov 7, 2024
@grorp grorp force-pushed the ogles2-fix branch 2 times, most recently from 635b6fd to 8067093 Compare November 7, 2024 19:44
grorp added 2 commits November 7, 2024 20:51
OES_depth32 only talks about support for render buffers, not textures,
so it's not relevant here:
https://github.com/KhronosGroup/OpenGL-Registry/blob/main/extensions/OES/OES_depth32.txt

This fixes the scene being black with "video_driver = ogles2" and
"enable_post_processing = true" on my desktop computer.
grepping for IRR_COMPILE_GLES2_COMMON gives no other results

COGLESCoreExtensionHandler is only used through COpenGL3ExtensionHandler
@grorp grorp changed the title [no sq] Don't claim support for ECF_D32 format with ogles2 driver [no sq] Fix ECF_D32 support in ogles2 video driver Nov 7, 2024
@grorp grorp requested a review from sfan5 November 7, 2024 19:53
Copy link
Collaborator

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

code looks good, no change on my pc

@sfan5
Copy link
Collaborator

sfan5 commented Nov 7, 2024

random thing I found on SO:

GL_DEPTH_COMPONENT32 is probably overkill, and as of 4.5, isn't a required format for OpenGL implementations to support (though 16 and 24 bit ones are). Consider that higher resolution depth buffers may not be as performant.

maybe we should be preferring ECF_D24S8 because do we really need 32-bit precision for the depth buffer?
even better we can add ECF_D24 to avoid wasting memory on a stencil buffer we don't need.

edit: also here:

Another great trick at the cost of some performance is to use a higher precision depth buffer. Most depth buffers have a precision of 24 bits, but most GPUs nowadays support 32 bit depth buffers, increasing the precision by a significant amount. So at the cost of some performance you'll get much more precision with depth testing, reducing z-fighting.

@grorp
Copy link
Member Author

grorp commented Nov 8, 2024

maybe we should be preferring ECF_D24S8 because do we really need 32-bit precision for the depth buffer?
even better we can add ECF_D24 to avoid wasting memory on a stencil buffer we don't need.

Luckily I can declare this "out-of-scope for this PR" 😅

@grorp grorp merged commit 50b7523 into luanti-org:master Nov 8, 2024
15 checks passed
@grorp grorp deleted the ogles2-fix branch November 8, 2024 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix 🐛 PRs that fix a bug @ Client rendering Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements One approval ✅ ◻️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Black scene with "ogles2" video driver + post-processing on desktop
2 participants