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.
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.
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).
(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...".
> + *
> + * 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
