On 01/09/2015 02:11 PM, michael.w.ma...@intel.com wrote: > From: Mike Mason <michael.w.ma...@intel.com>
The substance of this patch looks good to me. I have a few formatting nitpicks though. > Removes commit 7894278 changes and moves fix to _mesa_GetInternalformativ(). > The original commit enabled the GL_RGB and GL_RGBA unsized internal formats > as valid for render buffers in GLES3, but this is incorrect. They should > have only been enabled for GetInternalformativ() > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=88079 > --- > src/mesa/main/fbobject.c | 6 ------ > src/mesa/main/formatquery.c | 12 +++++++++++- > 2 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c > index 43b0886..f059750 100644 > --- a/src/mesa/main/fbobject.c > +++ b/src/mesa/main/fbobject.c > @@ -1430,9 +1430,6 @@ _mesa_base_fbo_format(struct gl_context *ctx, GLenum > internalFormat) > case GL_RGB8: > return GL_RGB; > case GL_RGB: > - if (_mesa_is_gles3(ctx)) > - return GL_RGB; > - /* fallthrough */ > case GL_R3_G3_B2: > case GL_RGB4: > case GL_RGB5: > @@ -1447,9 +1444,6 @@ _mesa_base_fbo_format(struct gl_context *ctx, GLenum > internalFormat) > case GL_RGBA8: > return GL_RGBA; > case GL_RGBA: > - if (_mesa_is_gles3(ctx)) > - return GL_RGBA; > - /* fallthrough */ > case GL_RGBA2: > case GL_RGBA12: > case GL_RGBA16: > diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c > index f6274fe..e8877c6 100644 > --- a/src/mesa/main/formatquery.c > +++ b/src/mesa/main/formatquery.c > @@ -89,8 +89,18 @@ _mesa_GetInternalformativ(GLenum target, GLenum > internalformat, GLenum pname, > * "If the <internalformat> parameter to GetInternalformativ is not > * color-, depth- or stencil-renderable, then an INVALID_ENUM error is > * generated." > + * > + * Page 243 of the GLES 3.0.4 spec says this for GetInternalformativ: > + * "internalformat must be color-renderable, depth-renderable or > + * stencilrenderable (as defined in section 4.4.4)." > + * Section 4.4.4 on page 212 of the same spec says: > + * "An internal format is color-renderable if it is one of the > + * formats from table 3.13 noted as color-renderable or if it > + * is unsized format RGBA or RGB." > + * Therefore, we must accept GL_RGB and GL_RGBA here. > */ Please add some newline padding aroundthe spec quotation. The usual style throughout Mesa, and elsewhere in this function, is: /* [The spec says] * [empty line] * [spec quote] * [empty line] * [more comments, if needed] */ The extra newlines make it easier to quickly visually parse the comments. Also, git complained about trailing whitespace in the hunk above. Here's the warning: ---- -8<- ---- Applying: mesa: Enable GL_RGB/GL_RGBA in GLES3 glGetInternalformativ /home/chadv/proj/mesa/work/.git/rebase-apply/patch:40: trailing whitespace. * "internalformat must be color-renderable, depth-renderable or warning: 1 line adds whitespace errors. ---- ->8- ---- If you don't see these warnings yourself, you can enable them by enabling the pre-commit hook distributed with git. $ cd ~/mesa/.git/hooks $ mv pre-commit.sample pre-commit Thanks for this bufgix!
signature.asc
Description: OpenPGP digital signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
