Yikes, apparently I missed one. Thanks for spotting that.
-- Chris
On Tue, Feb 12, 2013 at 3:35 PM, Eric Anholt <e...@anholt.net> wrote:
> Chris Forbes <chr...@ijw.co.nz> writes:
>
>> - sample count must be the same on all attachments
>> - fixedsamplepositions must be the same on all attachments
>> (renderbuffers have fixedsamplepositions=true implicitly; only
>> multisample textures can choose to have it false)
>>
>> V2: - fix wrapping to 80 columns, debug message, fix for state moving
>> from texobj to image.
>> - stencil texturing tweaks tidied up and folded in here.
>>
>> V3: - Removed silly stencil hacks entirely; the extension doesn't
>> actually make stencil-only textures legal at all.
>> - Moved sample count / fixed sample locations checks into
>> existing attachment-type-specific blocks, as suggested by Eric
>>
>> Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
>> [V2] Reviewed-by: Paul Berry <stereotype...@gmail.com>
>> ---
>> src/mesa/main/fbobject.c | 60
>> ++++++++++++++++++++++++++++++++++--------------
>> 1 file changed, 43 insertions(+), 17 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index d9fd78e..46663dd 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -664,8 +664,11 @@ test_attachment_completeness(const struct gl_context
>> *ctx, GLenum format,
>> baseFormat == GL_DEPTH_STENCIL_EXT) {
>> /* OK */
>> }
>> + else if (ctx->Extensions.ARB_texture_multisample &&
>> + baseFormat == GL_STENCIL_INDEX) {
>> + /* OK */
>> + }
>> else {
>> - /* no such thing as stencil-only textures */
>> att_incomplete("illegal stencil texture");
>> att->Complete = GL_FALSE;
>> return;
>
> Hey! You said you removed the silly stencil hacks! :)
>
>> @@ -745,6 +748,7 @@ _mesa_test_framebuffer_completeness(struct gl_context
>> *ctx,
>> GLenum intFormat = GL_NONE; /* color buffers' internal format */
>> GLuint minWidth = ~0, minHeight = ~0, maxWidth = 0, maxHeight = 0;
>> GLint numSamples = -1;
>> + GLint fixedSampleLocations = -1;
>> GLint i;
>> GLuint j;
>>
>> @@ -766,6 +770,7 @@ _mesa_test_framebuffer_completeness(struct gl_context
>> *ctx,
>> struct gl_renderbuffer_attachment *att;
>> GLenum f;
>> gl_format attFormat;
>> + struct gl_texture_image *texImg = NULL;
>>
>> /*
>> * XXX for ARB_fbo, only check color buffers that are named by
>> @@ -805,8 +810,7 @@ _mesa_test_framebuffer_completeness(struct gl_context
>> *ctx,
>> /* get width, height, format of the renderbuffer/texture
>> */
>> if (att->Type == GL_TEXTURE) {
>> - const struct gl_texture_image *texImg =
>> - _mesa_get_attachment_teximage(att);
>> + texImg = _mesa_get_attachment_teximage(att);
>> minWidth = MIN2(minWidth, texImg->Width);
>> maxWidth = MAX2(maxWidth, texImg->Width);
>> minHeight = MIN2(minHeight, texImg->Height);
>
> I think the texImg declaration change can be reverted now.
>
>> @@ -814,12 +818,29 @@ _mesa_test_framebuffer_completeness(struct gl_context
>> *ctx,
>> f = texImg->_BaseFormat;
>> attFormat = texImg->TexFormat;
>> numImages++;
>> +
>> if (!is_format_color_renderable(ctx, attFormat,
>> texImg->InternalFormat) &&
>> !is_legal_depth_format(ctx, f)) {
>> fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_FORMATS_EXT;
>> fbo_incomplete("texture attachment incomplete", -1);
>> return;
>> }
>> +
>> + if (numSamples < 0)
>> + numSamples = texImg->NumSamples;
>> + else if (numSamples != texImg->NumSamples) {
>> + fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE;
>> + fbo_incomplete("inconsistent sample count", -1);
>> + return;
>> + }
>> +
>> + if (fixedSampleLocations < 0)
>> + fixedSampleLocations = texImg->FixedSampleLocations;
>> + else if (fixedSampleLocations != texImg->FixedSampleLocations) {
>> + fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE;
>> + fbo_incomplete("inconsistent fixed sample locations", -1);
>> + return;
>> + }
>> }
>> else if (att->Type == GL_RENDERBUFFER_EXT) {
>> minWidth = MIN2(minWidth, att->Renderbuffer->Width);
>> @@ -829,24 +850,35 @@ _mesa_test_framebuffer_completeness(struct gl_context
>> *ctx,
>> f = att->Renderbuffer->InternalFormat;
>> attFormat = att->Renderbuffer->Format;
>> numImages++;
>> +
>> + if (numSamples < 0)
>> + numSamples = att->Renderbuffer->NumSamples;
>> + else if (numSamples != att->Renderbuffer->NumSamples) {
>> + fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE;
>> + fbo_incomplete("inconsistent sample count", -1);
>> + return;
>> + }
>> +
>> + /* RENDERBUFFER has fixedSampleLocations implicitly true */
>> + if (fixedSampleLocations < 0)
>> + fixedSampleLocations = GL_TRUE;
>> + else if (fixedSampleLocations != GL_TRUE) {
>> + fb->_Status = GL_FRAMEBUFFER_INCOMPLETE_MULTISAMPLE;
>> + fbo_incomplete("inconsistent fixed sample locations", -1);
>> + return;
>> + }
>
> This looks like what I was thinking.
>
> Other than the 2 trivial comments,
>
> Reviewed-by: Eric Anholt <e...@anholt.net>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
