[Mesa-dev] [RFC PATCH 06/10] i965/Gen4-5: Set clip flags from clip distances

[Chris Forbes]

You're right, that's completely bogus, and I meant to read the flags instead.

Thanks :)

On Tue, Aug 6, 2013 at 9:09 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 3 August 2013 19:59, Chris Forbes <chrisf at ijw.co.nz> wrote:
>>
>> Signed-off-by: Chris Forbes <chrisf at ijw.co.nz>
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 25
>> ++++++++++++++-----------
>>  1 file changed, 14 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index f80777b..c0b5ccd 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -2625,7 +2625,6 @@ vec4_visitor::emit_psiz_and_flags(struct brw_reg
>> reg)
>>        dst_reg header1 = dst_reg(this, glsl_type::uvec4_type);
>>        dst_reg header1_w = header1;
>>        header1_w.writemask = WRITEMASK_W;
>> -      GLuint i;
>>
>>        emit(MOV(header1, 0u));
>>
>> @@ -2637,18 +2636,22 @@ vec4_visitor::emit_psiz_and_flags(struct brw_reg
>> reg)
>>          emit(AND(header1_w, src_reg(header1_w), 0x7ff << 8));
>>        }
>>
>> -      current_annotation = "Clipping flags";
>> -      for (i = 0; i < key->nr_userclip_plane_consts; i++) {
>> -        vec4_instruction *inst;
>> -         gl_varying_slot slot = (prog_data->vue_map.slots_valid &
>> VARYING_BIT_CLIP_VERTEX)
>> -            ? VARYING_SLOT_CLIP_VERTEX : VARYING_SLOT_POS;
>> +      if (key->userclip_active) {
>> +         current_annotation = "Clipping flags";
>> +         dst_reg temp = dst_reg(this, glsl_type::uint_type);
>> +         dst_reg temp2 = dst_reg(this, glsl_type::uint_type);
>>
>> -        inst = emit(DP4(dst_null_f(), src_reg(output_reg[slot]),
>> -                         src_reg(this->userplane[i])));
>> -        inst->conditional_mod = BRW_CONDITIONAL_L;
>> +         emit(CMP(temp, src_reg(output_reg[VARYING_SLOT_CLIP_DIST0]),
>> src_reg(0.0f), BRW_CONDITIONAL_L));
>> +         emit(AND(temp2, src_reg(temp), src_reg(0x0fu)));
>
>
> I don't think this works.  From the i965 PRM
> (https://01.org/linuxgraphics/sites/default/files/documentation/965_g35_vol_4_subsystem_core_1.pdf),
> page 425:
>
> "Destination operand can be a GRF, an MRF or a null register. If it is not
> null, for the enabled channels, the LSB of the result in the destination
> channel contains the flag value for the channel. The other bits are
> undefined."
>
> You're making use of the lower 4 bits of the result, 3 of which are
> undefined.
>
> I believe you can achieve the effect you want by reading from the "flag"
> register, though you'll have to do some bit swizzling to unpack the values
> you want, since the flag register stores the result of the comparison for
> vertex 0 in its lower nibble and the result of the comparison for vertex 1
> in its lower nibble.
>
>>
>>
>> -        inst = emit(OR(header1_w, src_reg(header1_w), 1u << i));
>> -        inst->predicate = BRW_PREDICATE_NORMAL;
>> +         emit(CMP(temp, src_reg(output_reg[VARYING_SLOT_CLIP_DIST1]),
>> src_reg(0.0f), BRW_CONDITIONAL_L));
>> +         emit(AND(temp, src_reg(temp), src_reg(0x0fu)));
>> +         emit(SHL(temp, src_reg(temp), src_reg(4)));
>> +         emit(OR(temp2, src_reg(temp2), src_reg(temp)));
>> +
>> +         /* mask just the enabled planes */
>> +         emit(AND(temp2, src_reg(temp2),
>> src_reg(ctx->Transform.ClipPlanesEnabled)));
>> +         emit(OR(header1_w, src_reg(header1_w), src_reg(temp2)));
>>        }
>>
>>        /* i965 clipping workaround:
>> --
>> 1.8.3.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>