> -----Original Message----- > From: Versace, Chad > Sent: Tuesday, January 13, 2015 3:42 PM > To: Mason, Michael W; mesa-dev@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@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.ma...@intel.com wrote: > >>> From: Mike Mason <michael.w.ma...@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. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev