<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Nov 20, 2017 at 2:10 PM, Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Nov 20, 2017 at 01:54:39PM -0800, Nanley Chery wrote: > When sampling from fast-cleared sRGB textures on gen10, the hardware > will not decode the clear color to linear. We must therefore do it in > software. > I should've included the tests that are now passing (added in locally): * spec@arb_framebuffer_srgb@arb_framebuffer_srgb-fast-clear-blend * spec@arb_framebuffer_srgb@fbo-fast-clear * spec@arb_framebuffer_srgb@msaa-fast-clear * spec@ext_texture_srgb@fbo-fast-clear * spec@ext_texture_srgb@multisample-fast-clear gl_ext_texture_srgb -Nanley <div class="HOEnZb"><div class="h5"> > Signed-off-by: Nanley Chery <<a href="mailto:nanley.g.chery@intel.com">nanley.g.chery@intel.com</a>> > --- > src/mesa/drivers/dri/i965/brw_blorp.c | 7 +++++ > src/mesa/drivers/dri/i965/brw_meta_util.c | 38 ++++++++++++++++++++++++ > src/mesa/drivers/dri/i965/brw_meta_util.h | 5 ++++ > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 9 +++++- > 4 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c b/src/mesa/drivers/dri/i965/brw_blorp.c > index 38284d3b85..b10e9b95b7 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -328,6 +328,13 @@ brw_blorp_blit_miptrees(struct brw_context *brw, > struct blorp_surf src_surf, dst_surf; > blorp_surf_for_miptree(brw, &src_surf, src_mt, src_aux_usage, false, > &src_level, src_layer, 1, &tmp_surfs[0]); > + if (devinfo->gen >= 10 && isl_format_is_srgb(src_isl_format) && > + src_clear_supported) { > + src_surf.clear_color = > + gen10_convert_srgb_fast_clear_color(devinfo, src_isl_format, > + src_mt->fast_clear_color); > + } > + > blorp_surf_for_miptree(brw, &dst_surf, dst_mt, dst_aux_usage, true, > &dst_level, dst_layer, 1, &tmp_surfs[1]); > > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c b/src/mesa/drivers/dri/i965/brw_meta_util.c > index d292f5a8e2..97c6114f8a 100644 > --- a/src/mesa/drivers/dri/i965/brw_meta_util.c > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c > @@ -28,6 +28,7 @@ > #include "brw_state.h" > #include "main/blend.h" > #include "main/fbobject.h" > +#include "main/format_utils.h" > #include "util/format_srgb.h" > > /** > @@ -315,6 +316,43 @@ brw_is_color_fast_clear_compatible(struct brw_context *brw, > return true; > } > > +/* When sampling from fast-cleared sRGB textures on gen10, the hardware > + * will not decode the clear color to linear. We must therefore do it in > + * software. > + */ > +union isl_color_value > +gen10_convert_srgb_fast_clear_color(const struct gen_device_info *devinfo, > + enum isl_format format, > + const union isl_color_value color) > +{ > + assert(devinfo->gen >= 10); > + assert(isl_format_is_srgb(format)); > + assert(isl_format_supports_ccs_d(devinfo, format)); > + > + /* All the CCS-supported sRGB textures in GL are 4-channel, 8-bit/channel, > + * UNORM formats. > + */ > + assert(isl_format_get_num_channels(format) == 4); > + assert(isl_format_channels_have_size(format, 8)); > + assert(isl_format_has_unorm_channel(format)); </div></div></blockquote><div> </div><div>There are exactly 4 ISL formats which match this. You could just assert that it's one of the four and drop the other 5 asserts. It doesn't really matter, but this is rather complicated. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > + > + /* According to do_single_blorp_clear(), fast-clears for texture-views are > + * disabled. This means that we only have to do channel-to-channel format > + * conversions. > + */ > + union isl_color_value override_color = color; > + for (unsigned i = 0; i < 3; i++) { > + /* According to brw_is_color_fast_clear_compatible(), only floating-point > + * fast-clears have been enabled, so it is safe to rely on > + * isl_color_value::f32. > + */ </div></div></blockquote><div> </div><div>There is no such thing as integer sRGB, it wouldn't even make sense. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > + const uint8_t srgb_unorm8 = _mesa_float_to_unorm(color.f32[i], 8); > + override_color.f32[i] = > + util_format_srgb_8unorm_to_linear_float(srgb_unorm8); </div></div></blockquote><div> </div><div>Any particular reason why you're going through 8-bit? I can think of a few reasons why you might but I don't know if any of them actually matter in practice. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > + } > + return override_color; > +} > + > /** > * Convert the given color to a bitfield suitable for ORing into DWORD 7 of > * SURFACE_STATE (DWORD 12-15 on SKL+). > diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.h b/src/mesa/drivers/dri/i965/brw_meta_util.h > index 4b3408df15..ee0b3bd3e1 100644 > --- a/src/mesa/drivers/dri/i965/brw_meta_util.h > +++ b/src/mesa/drivers/dri/i965/brw_meta_util.h > @@ -42,6 +42,11 @@ brw_meta_mirror_clip_and_scissor(const struct gl_context *ctx, > GLfloat *dstX1, GLfloat *dstY1, > bool *mirror_x, bool *mirror_y); > > +union isl_color_value > +gen10_convert_srgb_fast_clear_color(const struct gen_device_info *devinfo, > + enum isl_format format, > + const union isl_color_value color); > + > union isl_color_value > brw_meta_convert_fast_clear_color(const struct brw_context *brw, > const struct intel_mipmap_tree *mt, > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > index adf60a840b..315b96e9c4 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > @@ -51,6 +51,7 @@ > #include "intel_buffer_objects.h" > > #include "brw_context.h" > +#include "brw_meta_util.h" > #include "brw_state.h" > #include "brw_defines.h" > #include "brw_wm.h" > @@ -174,7 +175,13 @@ brw_emit_surface_state(struct brw_context *brw, > /* We only really need a clear color if we also have an auxiliary > * surface. Without one, it does nothing. > */ > - clear_color = mt->fast_clear_color; > + if (devinfo->gen >= 10 && isl_format_is_srgb(view.format)) { > + clear_color = > + gen10_convert_srgb_fast_clear_color(devinfo, view.format, > + mt->fast_clear_color); > + } else { > + clear_color = mt->fast_clear_color; > + } > } > > void *state = brw_state_batch(brw, > -- > 2.14.3 > _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>