On Tue 21 Nov 2017, Jason Ekstrand wrote:
> On Tue, Nov 21, 2017 at 10:56 AM, Chad Versace <[1]chadvers...@chromium.org>
> wrote:
>
> On Wed 08 Nov 2017, Jason Ekstrand wrote:
> > This function is used to determine when we need to re-allocate a
> > miptree. Since we do nothing different in miptree allocation for
> > sRGB vs. linear, loosening this should be safe and may lead to less
> > copying and reallocating in some odd cases.
> >
> > Cc: "17.3" <[2]mesa-sta...@lists.freedesktop.org>
> > Cc: Kenneth Graunke <[3]kenn...@whitecape.org>
> > ---
> > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 6 ++++--
> > src/mesa/drivers/dri/i965/intel_tex.c | 2 +-
> > src/mesa/drivers/dri/i965/intel_tex_obj.h | 4 ++--
> > src/mesa/drivers/dri/i965/intel_tex_validate.c | 2 +-
> > 4 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/
> drivers/dri/i965/intel_mipmap_tree.c
> > index 82f5a81..47cfccc 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -1298,7 +1298,8 @@ intel_miptree_match_image(struct intel_mipmap_tree
> *mt,
> > if (mt->etc_format != MESA_FORMAT_NONE)
> > mt_format = mt->etc_format;
> >
> > - if (image->TexFormat != mt_format)
> > + if (_mesa_get_srgb_format_linear(image->TexFormat) !=
> > + _mesa_get_srgb_format_linear(mt_format))
> > return false;
> >
> > intel_get_image_dims(image, &width, &height, &depth);
> > @@ -1537,7 +1538,8 @@ intel_miptree_copy_slice(struct brw_context *brw,
> > assert(src_layer < get_num_phys_layers(&src_mt->surf,
> > src_level - src_mt->
> first_level));
> >
> > - assert(src_mt->format == dst_mt->format);
> > + assert(_mesa_get_srgb_format_linear(src_mt->format) ==
> > + _mesa_get_srgb_format_linear(dst_mt->format));
> >
> > if (dst_mt->compressed) {
> > unsigned int i, j;
> > diff --git a/src/mesa/drivers/dri/i965/intel_tex.c
> b/src/mesa/drivers/dri
> /i965/intel_tex.c
> > index 65a1cb3..0650b6e 100644
> > --- a/src/mesa/drivers/dri/i965/intel_tex.c
> > +++ b/src/mesa/drivers/dri/i965/intel_tex.c
> > @@ -176,7 +176,7 @@ intel_alloc_texture_storage(struct gl_context *ctx,
> > intel_texobj->needs_validate = false;
> > intel_texobj->validated_first_level = 0;
> > intel_texobj->validated_last_level = levels - 1;
> > - intel_texobj->_Format = intel_texobj->mt->format;
> > + intel_texobj->_Format = first_image->TexFormat;
>
> Why this line? Since all format comparisons, where appropriate, convert
> sRGB to linear pre-comparison, does it make any real difference whether
> intel_texobj->_Format is mt->format or first_image->TexFormat?
>
> It *feels* cleaner to use the incoming TexFormat here rather than
> mt->format. But I want to know if if this line was required to fix
> anything.
>
>
> Looking at the code again, I think it is. The way this block of code works is
> that the miptree gets its format from first_image and the texobj gets its
> format from the miptree. Previously, if the format changed between UNORM and
> sRGB, we would create a new miptree with the new format and the texobj would
> get the new format. With this change, we no longer create a new miptree but
> we
> still can't let the texture object and image formats get out of sync.
Ok. That makes sense.
>
>
> Due to interaction with below hunk from blorp, are we now disabling fast
> clears
> in more cases than previously?
>
> # brw_blorp.c:set_write_disables()
> /* We store clear colors as floats or uints as needed. If there
> are
> * texture views in play, the formats will not properly be
> respected
> * during resolves because the resolve operations only know about
> the
> * miptree and not the renderbuffer.
> */
> if (irb->Base.Base.Format != irb->mt->format)
> can_fast_clear = false;
>
>
> Ugh... Fortunately, I think that only happens in some rather obscure
> render-to-texture cases.
Even if this patch degrades some corner-case fast clears to slow clears,
the fixes in intel_mipmap_tree.c are worth it.
Patch 1/4 is
Reviewed-by: Chad Versace <chadvers...@chromium.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
