On 10/07/2013 04:31 PM, Kenneth Graunke wrote: > The previous code for sRGB overrides assumes that the source and > destination formats are equal, other than the color space. This won't > be feasible when we add support for format conversions. > > Here are a few cases, and how the old code handled them: > > 1. RGB8 -> SRGB8, MSAA ==> SRGB8 -> SRGB8 > 2. RGB8 -> SRGB8, single ==> RGB8 -> RGB8 > 3. SRGB8 -> RGB8, MSAA ==> RGB8 -> RGB8 > 4. SRGB8 -> RGB8, single ==> SRGB8 -> SRGB8 > > Apparently, preserving the behavior of #1 is important. When doing a > multisample to single-sample resolve, blending the samples together in > an sRGB correct fashion results in a noticably higher quality image. > It also is necessary to pass Piglit's EXT_framebuffer_multisample > accuracy color tests. > > Paul, Eric, Anuj, and I talked about this, and aren't sure that it > matters in the other cases. > > This patch preserves the behavior of #1, but otherwise reverts to > doing everything in linear space, changing the behavior of case #4. > > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/mesa/drivers/dri/i965/brw_blorp.cpp | 10 ++++++---- > src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 14 +++++++++----- > 2 files changed, 15 insertions(+), 9 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.cpp > b/src/mesa/drivers/dri/i965/brw_blorp.cpp > index c59bb66..91df346 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.cpp > +++ b/src/mesa/drivers/dri/i965/brw_blorp.cpp > @@ -104,15 +104,17 @@ brw_blorp_surface_info::set(struct brw_context *brw, > case MESA_FORMAT_Z16: > this->brw_surfaceformat = BRW_SURFACEFORMAT_R16_UNORM; > break; > - default: > + default: { > + gl_format linear_format = _mesa_get_srgb_format_linear(mt->format); > if (is_render_target) { > - assert(brw->format_supported_as_render_target[mt->format]); > - this->brw_surfaceformat = brw->render_target_format[mt->format]; > + assert(brw->format_supported_as_render_target[linear_format]); > + this->brw_surfaceformat = brw->render_target_format[linear_format]; > } else { > - this->brw_surfaceformat = brw_format_for_mesa_format(mt->format); > + this->brw_surfaceformat = brw_format_for_mesa_format(linear_format); > }
This is at least the second time this code block has appeared. inline member function? > break; > } > + } > } > > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > index f581871..2427085 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp > @@ -2077,13 +2077,17 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct > brw_context *brw, > * proprietary OpenGL driver also follow this approach. So, we choose to > * follow it in our driver. > * > - * Following if-else block takes care of this exception made for > - * multisampled resolves. > + * When multisampling, if the source and destination formats are equal > + * (aside from the color space), we choose to blit in sRGB space to get > + * this higher quality image. > */ > - if (src.num_samples > 1) > + if (src.num_samples > 1 && > + _mesa_get_format_color_encoding(dst_mt->format) == GL_SRGB && > + _mesa_get_srgb_format_linear(src_mt->format) == > + _mesa_get_srgb_format_linear(dst_mt->format)) { > + dst.brw_surfaceformat = brw_format_for_mesa_format(dst_mt->format); > src.brw_surfaceformat = dst.brw_surfaceformat; > - else > - dst.brw_surfaceformat = src.brw_surfaceformat; > + } > > use_wm_prog = true; > memset(&wm_prog_key, 0, sizeof(wm_prog_key)); > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev