| Message ID | 20160804231451.9994-10-kenneth@whitecape.org |
|---|---|
| State | New |
| Headers | show |
| Series |
"Series without cover letter"
( rev:
1
)
in
Piglit |
diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c index d6d3a42..8a79725 100644 --- a/src/mesa/drivers/common/meta_blit.c +++ b/src/mesa/drivers/common/meta_blit.c @@ -606,7 +606,6 @@ blitframebuffer_texture(struct gl_context *ctx, GLenum filter, GLint flipX, GLint flipY, GLboolean glsl_version, GLboolean do_depth) { - struct save_state *save = &ctx->Meta->Save[ctx->Meta->SaveStackDepth - 1]; int att_index = do_depth ? BUFFER_DEPTH : readFb->_ColorReadBufferIndex; const struct gl_renderbuffer_attachment *readAtt = &readFb->Attachment[att_index]; @@ -719,57 +718,32 @@ blitframebuffer_texture(struct gl_context *ctx, fb_tex_blit.samp_obj = _mesa_meta_setup_sampler(ctx, texObj, target, filter, srcLevel); - /* For desktop GL, we do our blits with no net sRGB decode or encode. - * - * However, if both the src and dst can be srgb decode/encoded, enable them - * so that we do any blending (from scaling or from MSAA resolves) in the - * right colorspace. - * - * Our choice of not doing any net encode/decode is from the GL 3.0 - * specification: - * - * "Blit operations bypass the fragment pipeline. The only fragment - * operations which affect a blit are the pixel ownership test and the - * scissor test." - * - * The GL 4.4 specification disagrees and says that the sRGB part of the - * fragment pipeline applies, but this was found to break applications - * (such as Left 4 Dead 2). - * - * However, for ES 3.0, we follow the specification and perform sRGB - * decoding and encoding. The specification has always been clear in - * the ES world, and hasn't changed over time. - */ if (ctx->Extensions.EXT_texture_sRGB_decode) { - bool src_srgb = _mesa_get_format_color_encoding(rb->Format) == GL_SRGB; - if (save->API == API_OPENGLES2 && ctx->Version >= 30) { - /* From the ES 3.0.4 specification, page 198: - * "When values are taken from the read buffer, if the value of - * FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer - * attachment corresponding to the read buffer is SRGB (see section - * 6.1.13), the red, green, and blue components are converted from - * the non-linear sRGB color space according to equation 3.24. - * - * When values are written to the draw buffers, blit operations - * bypass the fragment pipeline. The only fragment operations which - * affect a blit are the pixel ownership test, the scissor test, - * and sRGB conversion (see section 4.1.8)." - */ - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, - src_srgb ? GL_DECODE_EXT - : GL_SKIP_DECODE_EXT); - _mesa_set_framebuffer_srgb(ctx, drawFb->Visual.sRGBCapable); - } else { - if (src_srgb && drawFb->Visual.sRGBCapable) { - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, - GL_DECODE_EXT); - _mesa_set_framebuffer_srgb(ctx, GL_TRUE); - } else { - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, - GL_SKIP_DECODE_EXT); - /* set_framebuffer_srgb was set by _mesa_meta_begin(). */ - } - } + /* The GL 4.4 spec, section 18.3.1 ("Blitting Pixel Rectangles") says: + * + * "When values are taken from the read buffer, if FRAMEBUFFER_SRGB + * is enabled and the value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING + * for the framebuffer attachment corresponding to the read buffer + * is SRGB (see section 9.2.3), the red, green, and blue components + * are converted from the non-linear sRGB color space according to + * equation 3.24. + * + * When values are written to the draw buffers, blit operations + * bypass most of the fragment pipeline. The only fragment + * operations which affect a blit are the pixel ownership test, + * the scissor test, and sRGB conversion (see section 17.3.9)." + * + * ES 3.0 contains nearly the exact same text, but omits the part + * about GL_FRAMEBUFFER_SRGB as that doesn't exist in ES. Mesa + * defaults it to on for ES contexts, so we can safely check it. + */ + const bool decode = + ctx->Color.sRGBEnabled && + _mesa_get_format_color_encoding(rb->Format) == GL_SRGB; + + _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, + decode ? GL_DECODE_EXT + : GL_SKIP_DECODE_EXT); } if (!glsl_version) { @@ -1015,7 +989,9 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx, /* Only scissor affects blit, but we're doing to set a custom scissor if * necessary anyway, so save/clear state. */ - _mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS); + _mesa_meta_begin(ctx, MESA_META_ALL & + ~(MESA_META_DRAW_BUFFERS | + MESA_META_FRAMEBUFFER_SRGB)); /* Dithering shouldn't be performed for glBlitFramebuffer */ _mesa_set_enable(ctx, GL_DITHER, GL_FALSE);
On 08/04/2016 04:14 PM, Kenneth Graunke wrote: > Just avoid whacking GL_FRAMEBUFFER_SRGB altogether, so we respect > the application's setting. This appears to work. > > Signed-off-by: Kenneth Graunke <kenneth@whitecape.org> > --- > src/mesa/drivers/common/meta_blit.c | 80 +++++++++++++------------------------ > 1 file changed, 28 insertions(+), 52 deletions(-) > > Metaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!!! > > diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c > index d6d3a42..8a79725 100644 > --- a/src/mesa/drivers/common/meta_blit.c > +++ b/src/mesa/drivers/common/meta_blit.c > @@ -606,7 +606,6 @@ blitframebuffer_texture(struct gl_context *ctx, > GLenum filter, GLint flipX, GLint flipY, > GLboolean glsl_version, GLboolean do_depth) > { > - struct save_state *save = &ctx->Meta->Save[ctx->Meta->SaveStackDepth - 1]; > int att_index = do_depth ? BUFFER_DEPTH : readFb->_ColorReadBufferIndex; > const struct gl_renderbuffer_attachment *readAtt = > &readFb->Attachment[att_index]; > @@ -719,57 +718,32 @@ blitframebuffer_texture(struct gl_context *ctx, > fb_tex_blit.samp_obj = _mesa_meta_setup_sampler(ctx, texObj, target, filter, > srcLevel); > > - /* For desktop GL, we do our blits with no net sRGB decode or encode. > - * > - * However, if both the src and dst can be srgb decode/encoded, enable them > - * so that we do any blending (from scaling or from MSAA resolves) in the > - * right colorspace. > - * > - * Our choice of not doing any net encode/decode is from the GL 3.0 > - * specification: > - * > - * "Blit operations bypass the fragment pipeline. The only fragment > - * operations which affect a blit are the pixel ownership test and the > - * scissor test." > - * > - * The GL 4.4 specification disagrees and says that the sRGB part of the > - * fragment pipeline applies, but this was found to break applications > - * (such as Left 4 Dead 2). > - * > - * However, for ES 3.0, we follow the specification and perform sRGB > - * decoding and encoding. The specification has always been clear in > - * the ES world, and hasn't changed over time. > - */ > if (ctx->Extensions.EXT_texture_sRGB_decode) { > - bool src_srgb = _mesa_get_format_color_encoding(rb->Format) == GL_SRGB; > - if (save->API == API_OPENGLES2 && ctx->Version >= 30) { > - /* From the ES 3.0.4 specification, page 198: > - * "When values are taken from the read buffer, if the value of > - * FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING for the framebuffer > - * attachment corresponding to the read buffer is SRGB (see section > - * 6.1.13), the red, green, and blue components are converted from > - * the non-linear sRGB color space according to equation 3.24. > - * > - * When values are written to the draw buffers, blit operations > - * bypass the fragment pipeline. The only fragment operations which > - * affect a blit are the pixel ownership test, the scissor test, > - * and sRGB conversion (see section 4.1.8)." > - */ > - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, > - src_srgb ? GL_DECODE_EXT > - : GL_SKIP_DECODE_EXT); > - _mesa_set_framebuffer_srgb(ctx, drawFb->Visual.sRGBCapable); > - } else { > - if (src_srgb && drawFb->Visual.sRGBCapable) { > - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, > - GL_DECODE_EXT); > - _mesa_set_framebuffer_srgb(ctx, GL_TRUE); > - } else { > - _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, > - GL_SKIP_DECODE_EXT); > - /* set_framebuffer_srgb was set by _mesa_meta_begin(). */ > - } > - } > + /* The GL 4.4 spec, section 18.3.1 ("Blitting Pixel Rectangles") says: > + * > + * "When values are taken from the read buffer, if FRAMEBUFFER_SRGB > + * is enabled and the value of FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING > + * for the framebuffer attachment corresponding to the read buffer > + * is SRGB (see section 9.2.3), the red, green, and blue components > + * are converted from the non-linear sRGB color space according to > + * equation 3.24. > + * > + * When values are written to the draw buffers, blit operations > + * bypass most of the fragment pipeline. The only fragment > + * operations which affect a blit are the pixel ownership test, > + * the scissor test, and sRGB conversion (see section 17.3.9)." > + * > + * ES 3.0 contains nearly the exact same text, but omits the part > + * about GL_FRAMEBUFFER_SRGB as that doesn't exist in ES. Mesa > + * defaults it to on for ES contexts, so we can safely check it. > + */ > + const bool decode = > + ctx->Color.sRGBEnabled && > + _mesa_get_format_color_encoding(rb->Format) == GL_SRGB; > + > + _mesa_set_sampler_srgb_decode(ctx, fb_tex_blit.samp_obj, > + decode ? GL_DECODE_EXT > + : GL_SKIP_DECODE_EXT); > } > > if (!glsl_version) { > @@ -1015,7 +989,9 @@ _mesa_meta_BlitFramebuffer(struct gl_context *ctx, > /* Only scissor affects blit, but we're doing to set a custom scissor if > * necessary anyway, so save/clear state. > */ Update the comment too... and s/doing to set/going to set/. > - _mesa_meta_begin(ctx, MESA_META_ALL & ~MESA_META_DRAW_BUFFERS); > + _mesa_meta_begin(ctx, MESA_META_ALL & > + ~(MESA_META_DRAW_BUFFERS | > + MESA_META_FRAMEBUFFER_SRGB)); > > /* Dithering shouldn't be performed for glBlitFramebuffer */ > _mesa_set_enable(ctx, GL_DITHER, GL_FALSE); >
Just avoid whacking GL_FRAMEBUFFER_SRGB altogether, so we respect the application's setting. This appears to work. Signed-off-by: Kenneth Graunke <kenneth@whitecape.org> --- src/mesa/drivers/common/meta_blit.c | 80 +++++++++++++------------------------ 1 file changed, 28 insertions(+), 52 deletions(-) Metaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!!!