On Thu 12 Jul 2018, Jason Ekstrand wrote: > On Wed, Jun 27, 2018 at 5:55 PM Chad Versace <[1]chadvers...@chromium.org> > wrote: > > On Tue 26 Jun 2018, Jason Ekstrand wrote: > > At the moment, this is entirely internal but we'll expose it to clients > > of the BLORP API in the next commit. > > --- > > src/intel/blorp/blorp.h | 8 ++ > > src/intel/blorp/blorp_blit.c | 212 +++++++++++++++++++---------------- > > src/intel/blorp/blorp_priv.h | 12 +- > > 3 files changed, 123 insertions(+), 109 deletions(-) > > Yup, I still read this list. > > > \o/ > > > This patch makes the code easier to reason about. I like it. > > [snip] > > > + case BLORP_FILTER_BILINEAR: > > + assert(!key->src_tiled_w); > > + assert(key->tex_samples == key->src_samples); > > + assert(key->tex_layout == key->src_layout); > > What guarantees !key->src_tiled_w ? I can't deduce it from the patch. > > > That's stencil and you can't do a filtered scaled blit with stencil, only > nearest. I believe this is required/checked fairly high up in the GL API > area. > > > From my understanding of the patch, the patch allows the deduction > below. What is the missing step to !key->src_tiled_w? Does GL not allow > GL_LINEAR on stencil buffers? (If it does, though, then GL is dumb). > > > Correct. Which means that !stencil || LINEAR -> !stencil > > > (key.filter == BLORP_FILTER_BILINEAR) <-> ((blend && blit_scaled) || > bilinear_filter) > -> (blend || bilinear_filter) > -> (!(src_surf.usage & > ISL_SURF_USAGE_STENCIL_BIT) || (gl_filter == GL_LINEAR)) > ? > -> !stencil > -> !key->src_tiled_w > [snip] > > > + case BLORP_FILTER_AVERAGE: > > + assert(!key->src_tiled_w); > > + assert(key->tex_samples == key->src_samples); > > + assert(key->tex_layout == key->src_layout); > > + > > I expected to see assert(key->src_samples > 1) in this case. > Just an observation. > > [snip] > > > + /* We are downsampling a non-integer color buffer, so blend. > > This phrase is no longer inside an if. It should say "If we are..., > then blend.". Or "Blend if we are...". > > > I've changed it to "If we are..."
Ok. This patch 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
