KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

Should it be legal to do glRenderbufferStorage with a non-renderable format? #32

Closed imirkin closed 6 years ago

imirkin commented 6 years ago

Currently KHR-GL45.internalformat.renderbuffer.rgb9_e5 tests to ensure that you can do glRenderbufferStorage(GL_RGB9_E5) without regard to its renderability. [Note: NVIDIA hardware does not support rendering to this format.]

While GL_RGB9_E5 is indeed a color-renderable format, it's not marked as "Req. rend" in GL 4.6 core spec's table 8.12 (nor is it required per GL_EXT_texture_shared_exponent). Should it still be possible to create these renderbuffers, and then make the FB they're attached to incomplete? Or should we fail right when creating the RB?

[Note: current Mesa behavior is to fail creating the RB, which causes the test to fail. Trying to determine which behavior is correct.]

pdaniell-nv commented 6 years ago

We have a similar issue internally here: https://gitlab.khronos.org/Tracker/vk-gl-cts/issues/1002

dgkoch commented 6 years ago

As Piers noted, we've been discussing this inside Khronos related to a CTS issue (for the same test) that someone else from Mesa filed.

Quoting:

The RGB9_E5 is not a valid internalformat for RenderbufferStorage. From the OpenGL 4.6 
specification, 9.2.4 Renderbuffer Objects:

"An INVALID_ENUM error is generated if internalformat is not one of the color-renderable,
depth-renderable, or stencil-renderable formats defined in section 9.4."

This would be correct for a GL 4.6 core implementation that doesn't support EXT_texture_shared_exponent.

However, EXT_texture_shared_exponent clearly says that you can create RenderbufferStorage using RGB9_E5 (and has said that for almost 10 years, so I'd be hesitant to change it). However, as indicated in issue 8, it's still not actually going to be color-renderable in practise, and you'd get GL_FRAMEBUFFER_UNSUPPORTED_EXT when you tried to use this in an FBO. I think if you don't like doing that, you shouldn't expose the EXT_texture_shared_exponent extension.

imirkin commented 6 years ago

But even in GL 4.6, GL_RGB9_E5 is marked as "color renderable", but not "req rend". I guess I'm not 100% sure what "color renderable" means... is it "in the table AND actually renderable"? Or is it "in the table"?

Either way, sounds like the conclusion is that glRenderbufferStorage should return a crippled RB which would cause any FBO it's attached to to be incomplete. We can do that.

dgkoch commented 6 years ago

"CR" but not "Req Rend." means it's defined as a "color renderable" format, but it's not required to be supported as a rendering target (meaning it might only be supportable as a texture).

sounds like the conclusion is that glRenderbufferStorage should return a crippled RB which would cause any FBO it's attached to to be incomplete.

I'm pretty sure that's what we (NVIDIA) do.

imirkin commented 6 years ago

What if one uses glCopyImageSubData with such a RB? Do you get a INVALID_OPERATION, like you do with an incomplete texture? Or should it work effectively as a texture, and only cause failures when there's an FBO involved?

dgkoch commented 6 years ago

CR" but not "Req Rend." means it's defined as a "color renderable" format, but it's not required to be supported as a rendering target

Just to elaborate on this more - it also means that an implementation could support rendering to it if that is possible in the HW.

I don't see any reason why glCopyImageSubData wouldn't work, but I haven't personally tried that in our impl...

antiapuentes commented 6 years ago

Summing up, mainly for myself: (Referring to table 8.12, OpenGL 4.6)

For OpenGL 4.6 both columns "CR" and "Req. rend" are unmarked, so passing RGB9_E5 to RenderbufferStorage should return INVALID_ENUM. However, if EXT_texture_shared_exponent is exposed it would be like the "CR" column is marked and the format should be accepted.

This affects some of the Mesa Piglit ARB_internalformat_query2 tests expectations. In that regard, I assume that the COLOR_RENDERABLE query has the same meaning that the "CR" column explained above and it is affected by EXT_texture_shared_exponent exposure as well. In fact we already have the FRAMEBUFFER_RENDERABLE query to check if rendering is actually allowed.

Thanks!

pdaniell-nv commented 6 years ago

@imirkin Can we close this? Discussed in the WG call today, and we agree with the summary above.