On Fri, Oct 9, 2015 at 12:10 AM, Pohjolainen, Topi <topi.pohjolai...@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. --Jason >> >> /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */ >> if (!prog) >> @@ -209,12 +117,6 @@ brw_codegen_wm_prog(struct brw_context *brw, >> &prog_data.base); >> } >> >> - prog_data.barycentric_interp_modes = >> - brw_compute_barycentric_interp_modes(brw->intelScreen->devinfo, >> - key->flat_shade, >> - key->persample_shading, >> - fp->program.Base.nir); >> - >> if (unlikely(brw->perf_debug)) { >> start_busy = (brw->batch.last_bo && >> drm_intel_bo_busy(brw->batch.last_bo)); >> -- >> 2.5.0.400.gff86faf >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev