[Mesa-dev] [PATCH] mesa: Fix render buffer initial internal format type
Mason, Michael W
michael.w.mason at intel.com
Tue Jan 13 16:41:42 PST 2015
> -----Original Message-----
> From: Versace, Chad
> Sent: Tuesday, January 13, 2015 3:42 PM
> To: Mason, Michael W; mesa-dev at lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] mesa: Fix render buffer initial internal format type
>
> On 01/13/2015 02:34 PM, Mason, Michael W wrote:
> >> -----Original Message-----
> >> From: Versace, Chad
> >> Sent: Tuesday, January 13, 2015 10:47 AM
> >> To: Mason, Michael W; mesa-dev at lists.freedesktop.org
> >> Subject: Re: [Mesa-dev] [PATCH] mesa: Fix render buffer initial
> >> internal format type
> >>
> >> On 01/09/2015 05:21 PM, michael.w.mason at intel.com wrote:
> >>> From: Mike Mason <michael.w.mason at intel.com>
> >>>
> >>> Changes the initial internal format of a render buffer to GL_RGBA4.
> >>> This fixes a failure in the following DrawElements test:
> >>>
> >>> dEQP-GLES3.functional.state_query.rbo.renderbuffer_internal_format
> >>> ---
> >>> src/mesa/main/renderbuffer.c | 6 +++++-
> >>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/src/mesa/main/renderbuffer.c
> >>> b/src/mesa/main/renderbuffer.c index 0bc7f2b..b339491 100644
> >>> --- a/src/mesa/main/renderbuffer.c
> >>> +++ b/src/mesa/main/renderbuffer.c
> >>> @@ -53,7 +53,11 @@ _mesa_init_renderbuffer(struct gl_renderbuffer *rb, GLuint name)
> >>> rb->Width = 0;
> >>> rb->Height = 0;
> >>> rb->Depth = 0;
> >>> - rb->InternalFormat = GL_RGBA;
> >>> + /* Default internal format should be GL_RGBA4, per page 258,
> >>> + * table 6.15 of the GLES 3.0.4 spec. Same default is expected
> >>> + * in all OpenGL specs that support BindRenderbuffer().
> >>> + */
> >>> + rb->InternalFormat = GL_RGBA4;
> >>> rb->Format = MESA_FORMAT_NONE;
> >>> }
> >>>
> >>>
> >> The patch needs to choose the initial internalformat based on the
> >> context API. Table 6.26 in the GL 3.3 Core spec says the initial
> >> renderbuffer internalformat is GL_RGBA. Table 6.15 of the GLES 3.0 spec says GL_RGBA4.
> >>
> >> I think this is the correct logic:
> >>
> >> if (_mesa_is_desktop_gl(ctx)) {
> >> rb->InternalFormat = GL_RGBA;
> >> } else {
> >> rb->InternalFormat = GL_RGBA4;
> >> }
> >
> > "ctx" isn't available in _mesa_init_renderbuffer (where this code resides) and GET_CURRENT_CONTEXT(ctx) gives nil for
> ctx. Is there any other way to get a pointer to the gl_context?
>
> Drivers call _mesa_init_renderbuffer for 3 reasons, as far as I can tell:
>
> 1. To initialize Mesa's gl_renderbuffer structure in response to a real GL
> renderbuffer object created with glGenRenderbuffers.
>
> 2. To initialize a driver-private renderbuffer in response of the user
> attaching a texture to a framebuffer object, such as with glFramebufferTexture2D.
>
> 3. To initialize a driver-private renderbuffer that serves as backing storage
> for window system surfaces, such as eglCreateWindowSurface or a GLX Window.
>
> Only in case 1 will the user have a handle to the renderbuffer object and be able to call glGetRenderbufferParameteriv on
> it. In case 2, there is always a context present (I think...), but the driver will immediately overwrite the value of rb-
> >InternalFormat anyway, so it doesn't really matter what value _mesa_init_renderbuffer chooses. In case 3, there may or
> may not be a context present, but again the internalformat doesn't matter because the driver will immediately overwrite it.
>
> From that follows:
>
> - If a context is current, we are in case 1 or 2. Case 1 is the more restrictive
> case. It requires that we select the internalformat based on the context's API.
>
> - If a context is not current, we are in case 3, or possibly case 2 if I don't
> fully understand the problem space. Either way, in neither case 2 nor 3 does
> the value of internalformat matter, so we should just continue to initialize
> it to GL_RGBA as we have always done.
>
So would something like this be safe? Is ctx guaranteed to be either nil or a pointer to a valid context?
GET_CURRENT_CONTEXT(ctx);
if (ctx && _mesa_is_gles3(ctx)) {
rb->InternalFormat = GL_RGBA4;
} else {
rb->InternalFormat = GL_RGBA;
}
>
> >> Please add both spec references in your patch.
More information about the mesa-dev
mailing list