Fixed: BGFX_TEXTURE_MSAA_SAMPLE and msaa sampling bugs with gl #3351
Conversation
…sent in a framebuffer at once (sample locations were not fixed) I've just set the sample location as always fixed for now, although technically this can be false if only 1 msaa texture attachment is present
… time would result in an invalid framebuffer it would try to also resolve the sample-msaa texture at the same, leading to an invalid target type
this is valid, they don't resolving
parameter duplication on texelFetch(sampler2DMS,*) led to failed shaders
| @@ -954,7 +954,7 @@ void ir_print_glsl_visitor::visit(ir_texture *ir) | |||
| ir->coordinate->accept(this); | |||
|
|
|||
| // lod | |||
| if (ir->op == ir_txl || ir->op == ir_txf || ir->op == ir_txf_ms) | |||
There was a problem hiding this comment.
We handle the ir_txf_ms case a few lines below, so this lead to a duplicate last parameter with texelFetch(Sampler2DMS, ...)
| // if BGFX_TEXTURE_RT_MSAA_X2 or greater than BGFX_TEXTURE_RT_WRITE_ONLY is required | ||
| // if BGFX_TEXTURE_RT with no MSSA then WRITE_ONLY is not required. | ||
| // if BGFX_TEXTURE_RT_MSAA_X2 or greater than either BGFX_TEXTURE_RT_WRITE_ONLY or BGFX_TEXTURE_MSAA_SAMPLE is required | ||
| // if BGFX_TEXTURE_RT with no MSSA then this is not required. |
There was a problem hiding this comment.
BGFX_TEXTURE_MSAA_SAMPLER doesn't resolve either, so it's valid to use with depth attachments
| @@ -1479,7 +1479,7 @@ namespace bgfx { namespace gl | |||
| , _internalFormat | |||
| , _width | |||
| , _height | |||
| , false | |||
| , true | |||
There was a problem hiding this comment.
When we have multiple GL_TEXTURE_2D_MULTISAMPLE attachments this is required to be true, so that all sample locations match.
Realistically I don't know how common it is for sample locations to not match anyway, and I don't believe there is any penalty to doing so, so might as well have it on all the time for now.
| @@ -7101,7 +7101,7 @@ namespace bgfx { namespace gl | |||
| { | |||
| const TextureGL& texture = s_renderGL->m_textures[at.handle.idx]; | |||
|
|
|||
| if (0 != texture.m_id) | |||
| if (0 != texture.m_rbo && 0 != texture.m_id) | |||
There was a problem hiding this comment.
Don't try to resolve multisample textures too, ensure it also has an rbo!
(If one attachment required resolve, this would accidentally try and resolve the multisample texture too, causing an unhappy framebuffer)
ProPuke
changed the title
|
Did you test this with other backends? So that behavior is consistent? |
I haven't, no. |
|
Yeah, you should. |
BGFX_TEXTURE_MSAA_SAMPLE wasn't usable with multiple attachments.
Depth attachments couldn't be used with BGFX_TEXTURE_MSAA_SAMPLE.
Using texelFetch with Sampler2DMS would result in shader errors.