On Tue, Jul 19, 2016 at 08:26:12AM -0700, Jason Ekstrand wrote: > From the Sky Lake PRM: > > "For SURFTYPE_CUBE: For Sampling Engine Surfaces and Typed Data Port > Surfaces, the range of this field is [0,340], indicating the number of > cube array elements (equal to the number of underlying 2D array elements > divided by 6). For other surfaces, this field must be zero." > > In other words, the depth field for cube maps is in number of cubes not > number of 2-D slices so we need to divide by 6. It appears as if we've > been doing this wrong ever since we first added cube map arrays for Sandy > Bridge. Also, we now need to remove the shader hacks we've always done > since they were only needed because we were setting the depth field six > times too large. > > v2: Fix the vec4 backend as well (not sure how I missed this). > > Signed-off-by: Jason Ekstrand <[email protected]> > Cc: "12.0 11.2 11.1" <[email protected]> > --- > src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 21 +++++---------------- > src/mesa/drivers/dri/i965/brw_vec4.h | 1 - > src/mesa/drivers/dri/i965/brw_vec4_nir.cpp | 8 +------- > src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 15 ++++----------- > src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 6 +++++- > src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 3 ++- > src/mesa/drivers/dri/i965/gen8_surface_state.c | 3 ++- > 7 files changed, 19 insertions(+), 38 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > index 129984a..eeec0e2 100644 > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp > @@ -4423,26 +4423,15 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, > nir_tex_instr *instr) > for (unsigned i = 0; i < dest_size; i++) > nir_dest[i] = offset(dst, bld, i); > > - bool is_cube_array = instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE && > - instr->is_array; > - > if (instr->op == nir_texop_query_levels) { > /* # levels is in .w */ > nir_dest[0] = offset(dst, bld, 3); > - } else if (instr->op == nir_texop_txs && dest_size >= 3 && > - (devinfo->gen < 7 || is_cube_array)) { > + } else if (instr->op == nir_texop_txs && > + dest_size >= 3 && devinfo->gen < 7) { > + /* Gen4-6 return 0 instead of 1 for single layer surfaces. */ > fs_reg depth = offset(dst, bld, 2); > - fs_reg fixed_depth = vgrf(glsl_type::int_type); > - > - if (is_cube_array) { > - /* fixup #layers for cube map arrays */ > - bld.emit(SHADER_OPCODE_INT_QUOTIENT, fixed_depth, depth, > brw_imm_d(6)); > - } else if (devinfo->gen < 7) { > - /* Gen4-6 return 0 instead of 1 for single layer surfaces. */ > - bld.emit_minmax(fixed_depth, depth, brw_imm_d(1), > BRW_CONDITIONAL_GE); > - } > - > - nir_dest[2] = fixed_depth; > + nir_dest[2] = vgrf(glsl_type::int_type); > + bld.emit_minmax(nir_dest[2], depth, brw_imm_d(1), BRW_CONDITIONAL_GE); > } > > bld.LOAD_PAYLOAD(get_nir_dest(instr->dest), nir_dest, dest_size, 0); > diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h > b/src/mesa/drivers/dri/i965/brw_vec4.h > index 3043147..d544e71 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4.h > +++ b/src/mesa/drivers/dri/i965/brw_vec4.h > @@ -259,7 +259,6 @@ public: > uint32_t constant_offset, > src_reg offset_value, > src_reg mcs, > - bool is_cube_array, > uint32_t surface, src_reg surface_reg, > uint32_t sampler, src_reg sampler_reg); > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > index f3b4528..853a12b 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_nir.cpp > @@ -1875,16 +1875,10 @@ vec4_visitor::nir_emit_texture(nir_tex_instr *instr) > > ir_texture_opcode op = ir_texture_opcode_for_nir_texop(instr->op); > > - bool is_cube_array = > - instr->op == nir_texop_txs && > - instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE && > - instr->is_array; > - > emit_texture(op, dest, dest_type, coordinate, instr->coord_components, > shadow_comparitor, > lod, lod2, sample_index, > - constant_offset, offset_value, > - mcs, is_cube_array, > + constant_offset, offset_value, mcs, > texture, texture_reg, sampler, sampler_reg); > } > > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > index 652b453..b87d0a6 100644 > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp > @@ -907,7 +907,6 @@ vec4_visitor::emit_texture(ir_texture_opcode op, > uint32_t constant_offset, > src_reg offset_value, > src_reg mcs, > - bool is_cube_array, > uint32_t surface, > src_reg surface_reg, > uint32_t sampler, > @@ -1095,16 +1094,10 @@ vec4_visitor::emit_texture(ir_texture_opcode op, > /* fixup num layers (z) for cube arrays: hardware returns faces * layers; > * spec requires layers. > */ > - if (op == ir_txs) { > - if (is_cube_array) { > - emit_math(SHADER_OPCODE_INT_QUOTIENT, > - writemask(inst->dst, WRITEMASK_Z), > - src_reg(inst->dst), brw_imm_d(6)); > - } else if (devinfo->gen < 7) { > - /* Gen4-6 return 0 instead of 1 for single layer surfaces. */ > - emit_minmax(BRW_CONDITIONAL_GE, writemask(inst->dst, WRITEMASK_Z), > - src_reg(inst->dst), brw_imm_d(1)); > - } > + if (op == ir_txs && devinfo->gen < 7) { > + /* Gen4-6 return 0 instead of 1 for single layer surfaces. */ > + emit_minmax(BRW_CONDITIONAL_GE, writemask(inst->dst, WRITEMASK_Z), > + src_reg(inst->dst), brw_imm_d(1)); > } > > if (devinfo->gen == 6 && op == ir_tg4) { > diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > index c101e05..a96eae5 100644 > --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c > @@ -33,6 +33,7 @@ > #include "main/context.h" > #include "main/blend.h" > #include "main/mtypes.h" > +#include "main/teximage.h" > #include "main/samplerobj.h" > #include "main/shaderimage.h" > #include "program/prog_parameter.h" > @@ -360,8 +361,11 @@ brw_update_texture_surface(struct gl_context *ctx, > (mt->logical_width0 - 1) << BRW_SURFACE_WIDTH_SHIFT | > (mt->logical_height0 - 1) << BRW_SURFACE_HEIGHT_SHIFT); > > + const unsigned depth = mt->logical_depth0 / > + (_mesa_is_cube_map_texture(tObj->Target) ? 6 : 1);
In general, it starts to sound to me that logical_depth0 for cubes should also be the number of cubes instead of the number of faces. But I suppose this depends on point of view... _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
