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

Reading from depth textures #576

Closed
PeterHiber opened this issue Oct 28, 2021 · 12 comments
Closed

Reading from depth textures #576

PeterHiber opened this issue Oct 28, 2021 · 12 comments

Comments

@PeterHiber
Copy link

Hi again! Sorry in advance, we know that this is a duplicate of #165. But it's been some time, so basically we are creating this issue to see if anything has changed. Please feel free to close it if you wish :)

Being able to read from the depth texture is very useful. Its most obvious usage (shadow maps) can easily be worked around by writing the depth as a color in the pixel shader. But for post process effects (e.g. SSAO, depth of field, fog, etc) this is not really feasible, as you would have to render all geometry twice (in WebGL 1 without MRT). At least for us, our geometry pass is really expensive.

Looking around a bit, WEBGL_depth_texture seems well supported on basically everything as far as I can tell. But it seems the problem is more with SPIRV-Cross and sokol_shdc? If it helps anything, we don't necessarily need the whole "sample depth texture, compare it with reference value and return 0 or 1 depending on comparison" thing. We just want to be able to sample the raw depth with nearest filtering.

Is there anything we can do to help to make this happen? :)

@floooh
Copy link
Owner

floooh commented Nov 9, 2021

Haven't looked into this yet, sorry. I think the best way forward would be to gate the issue behind a new feature flag (sg_query_features) so that support can be added gradually to the backends instead of having to touch all backends at once.

For GL, these parts would need to be touched:

  • GLES2/WebGL needs an extension check here:

    sokol/sokol_gfx.h

    Lines 5659 to 5699 in 8fcd271

    for (int i = 0; i < num_ext; i++) {
    const char* ext = (const char*) glGetStringi(GL_EXTENSIONS, (GLuint)i);
    if (ext) {
    if (strstr(ext, "_texture_compression_s3tc")) {
    has_s3tc = true;
    }
    else if (strstr(ext, "_compressed_texture_s3tc")) {
    has_s3tc = true;
    }
    else if (strstr(ext, "_texture_compression_rgtc")) {
    has_rgtc = true;
    }
    else if (strstr(ext, "_texture_compression_bptc")) {
    has_bptc = true;
    }
    else if (strstr(ext, "_texture_compression_pvrtc")) {
    has_pvrtc = true;
    }
    else if (strstr(ext, "_compressed_texture_pvrtc")) {
    has_pvrtc = true;
    }
    else if (strstr(ext, "_compressed_texture_etc")) {
    has_etc2 = true;
    }
    else if (strstr(ext, "_color_buffer_float")) {
    has_colorbuffer_float = true;
    }
    else if (strstr(ext, "_color_buffer_half_float")) {
    has_colorbuffer_half_float = true;
    }
    else if (strstr(ext, "_texture_float_linear")) {
    has_texture_float_linear = true;
    }
    else if (strstr(ext, "_float_blend")) {
    has_float_blend = true;
    }
    else if (strstr(ext, "_texture_filter_anisotropic")) {
    _sg.gl.ext_anisotropic = true;
    }
    }
    }

  • in _sg_gl_create_texture() the special case block for depth-stencil-textures should be skipped if WEBGL_depth_texture is supported:

    sokol/sokol_gfx.h

    Lines 6237 to 6251 in 8fcd271

    /* special case depth-stencil-buffer? */
    SOKOL_ASSERT((img->cmn.usage == SG_USAGE_IMMUTABLE) && (img->cmn.num_slots == 1));
    SOKOL_ASSERT(!img->gl.ext_textures); /* cannot provide external texture for depth images */
    glGenRenderbuffers(1, &img->gl.depth_render_buffer);
    glBindRenderbuffer(GL_RENDERBUFFER, img->gl.depth_render_buffer);
    GLenum gl_depth_format = _sg_gl_depth_attachment_format(img->cmn.pixel_format);
    #if !defined(SOKOL_GLES2)
    if (!_sg.gl.gles2 && msaa) {
    glRenderbufferStorageMultisample(GL_RENDERBUFFER, img->cmn.sample_count, gl_depth_format, img->cmn.width, img->cmn.height);
    }
    else
    #endif
    {
    glRenderbufferStorage(GL_RENDERBUFFER, gl_depth_format, img->cmn.width, img->cmn.height);
    }

  • and this part in _sg_gl_create_pass() needs to be fixed accordingly:

    sokol/sokol_gfx.h

    Lines 6706 to 6714 in 8fcd271

    /* attach depth-stencil buffer to framebuffer */
    if (pass->gl.ds_att.image) {
    const GLuint gl_render_buffer = pass->gl.ds_att.image->gl.depth_render_buffer;
    SOKOL_ASSERT(gl_render_buffer);
    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, gl_render_buffer);
    if (_sg_is_depth_stencil_format(pass->gl.ds_att.image->cmn.pixel_format)) {
    glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER, gl_render_buffer);
    }
    }

  • IMHO the old render buffer code should still remain as fallback for WebGL1 (but hopefully we can ditch WebGL1 support in the not-too-distant future)

(also note the special case handling for MSAA framebuffers, this needs to be handled the same for depth-textures)

If you want to create such a PR limited to the GL backend I'm happy to merge :)

(I think support for the other backends will be much easier)

@floooh
Copy link
Owner

floooh commented Nov 9, 2021

PS:

But it seems the problem is more with SPIRV-Cross and sokol_shdc?

I'm not aware of any issues there so far, hopefully sokol_shdc and SPIRV-Cross will support depth texture operations out of the box.

@floooh
Copy link
Owner

floooh commented Nov 9, 2021

PPS: As you have noticed I'm currently a bit unresponsive regarding sokol issues, reason is that I'm currently deep in the emulator-coding rabbit hole again :)

@PeterHiber
Copy link
Author

No problem! :) Hopefully we'll have time to take a look at this sometime soon. Sadly can't promise anything though, our priorities tend to change a lot and very quickly. 😅

@PeterHiber
Copy link
Author

We have started working on this, and if all goes well we'll have a PR up in the near future (end of next week maybe). Will have to warn though that sadly there is a good chance that we won't have time to properly clean up the implementation, so there's a risk that the PR will be a bit dirty.

@floooh
Copy link
Owner

floooh commented Nov 18, 2021

No problem, worst case is that the PR will be "parked" for a little while until I get around to clean it up a bit :)

@pseregiet
Copy link

I would be really happy if this is implemented. I just started programming in 3D and use sokol_gfx and when I got into shadow mapping I discovered I have to do this workaround for a depth buffer... Obviously it works but if it can be done in a "normal" way it would be awesome.

@PeterHiber
Copy link
Author

It's been a while (and well, priorities change a lot), but we have managed to get something working on a few targets. Works fine on D3D11 and WebGL 2. We have some issues getting it to work on WebGL 1 even with the depth_texture extension (iOS 14). Metal has so far turned out to be the hardest target, and haven't gotten it to work there at all.

For Metal we suspect that it might be necessary to modify sokol_shdc. The problem is that MSL has a different type for depth textures (depth2d) versus normal ones (texture2d), this is not the case in either GLSL or HLSL.

We are going to spend some more time today, but likely we will put up a PR later today with what we have. Hopefully someone can help fix the remaining issues. :)

@pseregiet
Copy link

pseregiet commented Jan 4, 2022

@PeterHiber Do you have a change that would support at least openGL 3.3 ? Would really appreciate.

Unless your experimental PR already supports openGL3.3 and I'm just using it wrong. When I create a render pass without a color attachment I get an error about it

    shadow.tpip = sg_make_pipeline(&(sg_pipeline_desc){
        .shader = shd_depth,
        .index_type = SG_INDEXTYPE_UINT16,
        .layout = {
            .buffers[ATTR_vs_depth_apos] = {.stride = 11 * sizeof(float) },
            .attrs[ATTR_vs_depth_apos] = {.format = SG_VERTEXFORMAT_FLOAT3},
        },
        .depth = {
            .pixel_format = SG_PIXELFORMAT_DEPTH,
            .compare = SG_COMPAREFUNC_LESS_EQUAL,
            .write_enabled = true,
        },
        //.color_count = 1,
        //.colors[0] = {
        //    .write_mask = SG_COLORMASK_RGB
        //},
        .cull_mode = SG_CULLMODE_FRONT,
    });

    shadow.pass = sg_make_pass(&(sg_pass_desc) {
        //.color_attachments[0].image = shadow.colormap,
        .depth_stencil_attachment.image = shadow.depthmap,
    });

sg_pass_desc.color_attachments[0] must be valid

If I put an empty texture as color_attachment[0] and not use it in a shader it still works, I can pass a depth buffer to another shader and it works.

@PeterHiber
Copy link
Author

@pseregiet

Sorry, I thought OpenGL 3.3 was supported, but must have missed some specific codepath for that API. OpenGL 3.3 is the only API sokol_gfx has a backend for that we are not using internally, so I never tested it.

@pseregiet
Copy link

@PeterHiber Well, I can read from a depth texture without any problems but when I want to render to a depth texture I must attach a dummy RGB texture for a color attachment.

@floooh
Copy link
Owner

floooh commented Apr 28, 2023

Closing this issue (a bit preliminary) as fixed, once this PR (#821, which removes GLES2/WebGL1 support) is merged, depth textures will be regular textures in the GL backend (not render buffers as before).

There's still some missing feature (when a render target is MSAA, the depth MSAA render target will not be resolved, and also discarded) - this will be fixed with this issue: #816).

And even after that there will still be an issue with MSAA depth render targets. The depth MSAA surface will still not be a regular texture in the GL backend, but a render buffer (since MSAA textures don't seem to be supported in GLES3), so it won't be possible to directly sample an MSAA surface in shaders - and it's currently unclear to me if an MSAA-resolved texture would be any useful (because AFAIK the resolve would average the depth samples) - this might be something that will be possible in the other backends, but not GL (I need to figure out how to communicate this in the API - probably needs to be a new boolean in sg_pixelformat_info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants