On Wed, Jun 27, 2018 at 5:55 PM Chad Versace <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..." > > + * > > + * Regarding integer color buffers, the OpenGL ES 3.2 spec says: > > + * > > + * "If the source formats are integer types or stencil values, a > > + * single sample's value is selected for each pixel." > > + * > > + * This implies we should not blend in that case. > > + */ > > + const bool blend = > > + (params.src.surf.usage & ISL_SURF_USAGE_DEPTH_BIT) == 0 && > > + (params.src.surf.usage & ISL_SURF_USAGE_STENCIL_BIT) == 0 && > > + !isl_format_has_int_channel(params.src.surf.format) && > > + params.src.surf.samples > 1 && > > + params.dst.surf.samples <= 1; >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
