[Mesa-dev] [PATCH 15/17] i965/fs: Move some of the prog_data setup into brw_wm_emit
Pohjolainen, Topi
topi.pohjolainen at intel.com
Fri Oct 16 00:35:13 PDT 2015
On Fri, Oct 09, 2015 at 05:50:22AM -0700, Jason Ekstrand wrote:
> On Fri, Oct 9, 2015 at 12:10 AM, Pohjolainen, Topi
> <topi.pohjolainen at intel.com> wrote:
> > On Thu, Oct 08, 2015 at 05:22:47PM -0700, Jason Ekstrand wrote:
> >> This commit moves the common/modern stuff. Some legacy stuff such as
> >> setting use_alt_mode was left because it needs to know whether or not we're
> >> an ARB program.
> >> ---
> >> src/mesa/drivers/dri/i965/brw_fs.cpp | 98 ++++++++++++++++++++++++++++++++++++
> >> src/mesa/drivers/dri/i965/brw_wm.c | 98 ------------------------------------
> >> 2 files changed, 98 insertions(+), 98 deletions(-)
> >>
> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> index 146f4b4..0e39b50 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> >> @@ -5114,6 +5114,90 @@ fs_visitor::run_cs()
> >> return !failed;
> >> }
> >>
> >> +/**
> >> + * Return a bitfield where bit n is set if barycentric interpolation mode n
> >> + * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
> >> + */
> >> +static unsigned
> >> +brw_compute_barycentric_interp_modes(const struct brw_device_info *devinfo,
> >> + bool shade_model_flat,
> >> + bool persample_shading,
> >> + const nir_shader *shader)
> >> +{
> >> + unsigned barycentric_interp_modes = 0;
> >> +
> >> + nir_foreach_variable(var, &shader->inputs) {
> >> + enum glsl_interp_qualifier interp_qualifier =
> >> + (enum glsl_interp_qualifier)var->data.interpolation;
> >> + bool is_centroid = var->data.centroid && !persample_shading;
> >> + bool is_sample = var->data.sample || persample_shading;
> >> + bool is_gl_Color = (var->data.location == VARYING_SLOT_COL0) ||
> >> + (var->data.location == VARYING_SLOT_COL1);
> >> +
> >> + /* Ignore WPOS and FACE, because they don't require interpolation. */
> >> + if (var->data.location == VARYING_SLOT_POS ||
> >> + var->data.location == VARYING_SLOT_FACE)
> >> + continue;
> >> +
> >> + /* Determine the set (or sets) of barycentric coordinates needed to
> >> + * interpolate this variable. Note that when
> >> + * brw->needs_unlit_centroid_workaround is set, centroid interpolation
> >> + * uses PIXEL interpolation for unlit pixels and CENTROID interpolation
> >> + * for lit pixels, so we need both sets of barycentric coordinates.
> >> + */
> >> + if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
> >> + if (is_centroid) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
> >> + } else if (is_sample) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC;
> >> + }
> >> + if ((!is_centroid && !is_sample) ||
> >> + devinfo->needs_unlit_centroid_workaround) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
> >> + }
> >> + } else if (interp_qualifier == INTERP_QUALIFIER_SMOOTH ||
> >> + (!(shade_model_flat && is_gl_Color) &&
> >> + interp_qualifier == INTERP_QUALIFIER_NONE)) {
> >> + if (is_centroid) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
> >> + } else if (is_sample) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC;
> >> + }
> >> + if ((!is_centroid && !is_sample) ||
> >> + devinfo->needs_unlit_centroid_workaround) {
> >> + barycentric_interp_modes |=
> >> + 1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
> >> + }
> >> + }
> >> + }
> >> +
> >> + return barycentric_interp_modes;
> >> +}
> >> +
> >> +static uint8_t
> >> +computed_depth_mode(const nir_shader *shader)
> >> +{
> >> + if (shader->info.outputs_written & BITFIELD64_BIT(FRAG_RESULT_DEPTH)) {
> >> + switch (shader->info.fs.depth_layout) {
> >> + case FRAG_DEPTH_LAYOUT_NONE:
> >> + case FRAG_DEPTH_LAYOUT_ANY:
> >> + return BRW_PSCDEPTH_ON;
> >> + case FRAG_DEPTH_LAYOUT_GREATER:
> >> + return BRW_PSCDEPTH_ON_GE;
> >> + case FRAG_DEPTH_LAYOUT_LESS:
> >> + return BRW_PSCDEPTH_ON_LE;
> >> + case FRAG_DEPTH_LAYOUT_UNCHANGED:
> >> + return BRW_PSCDEPTH_OFF;
> >> + }
> >> + }
> >> + return BRW_PSCDEPTH_OFF;
> >> +}
> >> +
> >> const unsigned *
> >> brw_wm_fs_emit(const struct brw_compiler *compiler, void *log_data,
> >> void *mem_ctx,
> >> @@ -5126,6 +5210,20 @@ brw_wm_fs_emit(const struct brw_compiler *compiler, void *log_data,
> >> unsigned *final_assembly_size,
> >> char **error_str)
> >> {
> >> + /* key->alpha_test_func means simulating alpha testing via discards,
> >> + * so the shader definitely kills pixels.
> >> + */
> >> + prog_data->uses_kill = shader->info.fs.uses_discard || key->alpha_test_func;
> >> + prog_data->uses_omask =
> >> + shader->info.outputs_written & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK);
> >> + prog_data->computed_depth_mode = computed_depth_mode(shader);
> >> +
> >> + prog_data->barycentric_interp_modes =
> >> + brw_compute_barycentric_interp_modes(compiler->devinfo,
> >> + key->flat_shade,
> >> + key->persample_shading,
> >> + shader);
> >> +
> >> fs_visitor v(compiler, log_data, mem_ctx, key,
> >> &prog_data->base, prog, shader, 8,
> >> shader_time_index8);
> >> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> >> index ab9461a..91bda35 100644
> >> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> >> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> >> @@ -39,89 +39,6 @@
> >>
> >> #include "util/ralloc.h"
> >>
> >> -/**
> >> - * Return a bitfield where bit n is set if barycentric interpolation mode n
> >> - * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
> >> - */
> >> -static unsigned
> >> -brw_compute_barycentric_interp_modes(const struct brw_device_info *devinfo,
> >> - bool shade_model_flat,
> >> - bool persample_shading,
> >> - nir_shader *shader)
> >> -{
> >> - unsigned barycentric_interp_modes = 0;
> >> -
> >> - nir_foreach_variable(var, &shader->inputs) {
> >> - enum glsl_interp_qualifier interp_qualifier = var->data.interpolation;
> >> - bool is_centroid = var->data.centroid && !persample_shading;
> >> - bool is_sample = var->data.sample || persample_shading;
> >> - bool is_gl_Color = (var->data.location == VARYING_SLOT_COL0) ||
> >> - (var->data.location == VARYING_SLOT_COL1);
> >> -
> >> - /* Ignore WPOS and FACE, because they don't require interpolation. */
> >> - if (var->data.location == VARYING_SLOT_POS ||
> >> - var->data.location == VARYING_SLOT_FACE)
> >> - continue;
> >> -
> >> - /* Determine the set (or sets) of barycentric coordinates needed to
> >> - * interpolate this variable. Note that when
> >> - * brw->needs_unlit_centroid_workaround is set, centroid interpolation
> >> - * uses PIXEL interpolation for unlit pixels and CENTROID interpolation
> >> - * for lit pixels, so we need both sets of barycentric coordinates.
> >> - */
> >> - if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
> >> - if (is_centroid) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
> >> - } else if (is_sample) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_NONPERSPECTIVE_SAMPLE_BARYCENTRIC;
> >> - }
> >> - if ((!is_centroid && !is_sample) ||
> >> - devinfo->needs_unlit_centroid_workaround) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
> >> - }
> >> - } else if (interp_qualifier == INTERP_QUALIFIER_SMOOTH ||
> >> - (!(shade_model_flat && is_gl_Color) &&
> >> - interp_qualifier == INTERP_QUALIFIER_NONE)) {
> >> - if (is_centroid) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
> >> - } else if (is_sample) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_PERSPECTIVE_SAMPLE_BARYCENTRIC;
> >> - }
> >> - if ((!is_centroid && !is_sample) ||
> >> - devinfo->needs_unlit_centroid_workaround) {
> >> - barycentric_interp_modes |=
> >> - 1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
> >> - }
> >> - }
> >> - }
> >> -
> >> - return barycentric_interp_modes;
> >> -}
> >> -
> >> -static uint8_t
> >> -computed_depth_mode(struct gl_fragment_program *fp)
> >> -{
> >> - if (fp->Base.OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_DEPTH)) {
> >> - switch (fp->FragDepthLayout) {
> >> - case FRAG_DEPTH_LAYOUT_NONE:
> >> - case FRAG_DEPTH_LAYOUT_ANY:
> >> - return BRW_PSCDEPTH_ON;
> >> - case FRAG_DEPTH_LAYOUT_GREATER:
> >> - return BRW_PSCDEPTH_ON_GE;
> >> - case FRAG_DEPTH_LAYOUT_LESS:
> >> - return BRW_PSCDEPTH_ON_LE;
> >> - case FRAG_DEPTH_LAYOUT_UNCHANGED:
> >> - return BRW_PSCDEPTH_OFF;
> >> - }
> >> - }
> >> - return BRW_PSCDEPTH_OFF;
> >> -}
> >> -
> >> static void
> >> assign_fs_binding_table_offsets(const struct brw_device_info *devinfo,
> >> const struct gl_shader_program *shader_prog,
> >> @@ -166,15 +83,6 @@ brw_codegen_wm_prog(struct brw_context *brw,
> >> fs = (struct brw_shader *)prog->_LinkedShaders[MESA_SHADER_FRAGMENT];
> >>
> >> memset(&prog_data, 0, sizeof(prog_data));
> >> - /* key->alpha_test_func means simulating alpha testing via discards,
> >> - * so the shader definitely kills pixels.
> >> - */
> >> - prog_data.uses_kill = fp->program.UsesKill || key->alpha_test_func;
> >> - prog_data.uses_omask =
> >> - fp->program.Base.OutputsWritten & BITFIELD64_BIT(FRAG_RESULT_SAMPLE_MASK);
> >> - prog_data.computed_depth_mode = computed_depth_mode(&fp->program);
> >> -
> >> - prog_data.early_fragment_tests = fs && fs->base.EarlyFragmentTests;
> >
> > You store "early_fragment_tests" in patch 6 into shader info but this patch
> > doesn't add logic to read that and set it for prog_data (I was expecting to
> > see that in brw_wm_fs_emit()). To me it seems that the corresponding field
> > in prog_data is now always left to false.
>
> Good catch! You're absolutely correct. Some how that got dropped in
> my copy-and-pasting. I've added
>
> + prog_data->early_fragment_tests = shader->info.fs.early_fragment_tests;
>
> to the hunk above where I set the other prog_data stuff.
Sounds right, this patch and the remaining 6, 11, 13, 14 and 16 are also:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
(There is no patch number 17 in the series, is there?)
More information about the mesa-dev
mailing list