[virglrenderer-devel] [PATCHv2 virglrenderer] shader: improve TXQ translation

[Dave Airlie]

On 29 February 2016 at 09:38, Marc-André Lureau
<marcandre.lureau at gmail.com> wrote:
> Hi
>
> On Fri, Feb 26, 2016 at 10:46 PM, Dave Airlie <airlied at gmail.com> wrote:
>> On 27 February 2016 at 04:48, Marc-André Lureau
>> <marcandre.lureau at gmail.com> wrote:
>>> Hi
>>>
>>> On Fri, Feb 26, 2016 at 5:43 AM, Dave Airlie <airlied at gmail.com> wrote:
>>>> On 22 February 2016 at 09:39,  <marcandre.lureau at redhat.com> wrote:
>>>>> From: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>>>
>>>>> TXQ may query levels and size.
>>>>>
>>>>> Fixes all piglit texturesize tests and others.
>>>>>
>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
>>>>> ---
>>>>>  src/vrend_shader.c | 50 +++++++++++++++++++++++++++++++++++++++++++++-----
>>>>>  1 file changed, 45 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/src/vrend_shader.c b/src/vrend_shader.c
>>>>> index e72231f..ebf53a1 100644
>>>>> --- a/src/vrend_shader.c
>>>>> +++ b/src/vrend_shader.c
>>>>> @@ -25,6 +25,7 @@
>>>>>  #include "tgsi/tgsi_info.h"
>>>>>  #include "tgsi/tgsi_iterate.h"
>>>>>  #include "util/u_memory.h"
>>>>> +#include "util/u_math.h"
>>>>>  #include <string.h>
>>>>>  #include <stdio.h>
>>>>>  #include <math.h>
>>>>> @@ -1008,17 +1009,56 @@ static int translate_tex(struct dump_ctx *ctx,
>>>>>            inst->Texture.Texture != TGSI_TEXTURE_2D_ARRAY_MSAA)
>>>>>           snprintf(bias, 128, ", int(%s.w)", srcs[0]);
>>>>>
>>>>> +      twm = "";
>>>>>        /* need to emit a textureQueryLevels */
>>>>>        if (inst->Dst[0].Register.WriteMask & 0x8) {
>>>>> -         ctx->uses_txq_levels = true;
>>>>> -         snprintf(buf, 255, "%s = %s(%s(textureQueryLevels(%s)));\n", dsts[0], dstconv, dtypeprefix, srcs[sampler_index]);
>>>>> -         return emit_buf(ctx, buf);
>>>>> -      } else {
>>>>> -         snprintf(buf, 255, "%s = %s(%s(textureSize(%s%s)));\n", dsts[0], dstconv, dtypeprefix, srcs[sampler_index], bias);
>>>>> +
>>>>> +         if (inst->Texture.Texture != TGSI_TEXTURE_BUFFER &&
>>>>> +             inst->Texture.Texture != TGSI_TEXTURE_RECT &&
>>>>> +             inst->Texture.Texture != TGSI_TEXTURE_2D_MSAA &&
>>>>> +             inst->Texture.Texture != TGSI_TEXTURE_2D_ARRAY_MSAA) {
>>>>> +            ctx->uses_txq_levels = true;
>>>>> +            if (inst->Dst[0].Register.WriteMask & 0x7)
>>>>> +               twm = ".w";
>>>>> +            snprintf(buf, 255, "%s%s = %s(textureQueryLevels(%s));\n", dsts[0], twm, dtypeprefix, srcs[sampler_index]);
>>>>> +            emit_buf(ctx, buf);
>>>>
>>>> shouldn't this return here? ^^
>>>>
>>>> else we emit another texture instruction after the TXQ.
>>>
>>> http://gallium.readthedocs.org/en/latest/tgsi.html#opcode-TXQ is not
>>> only about levels.
>>
>> So if we don't have any writemasks for XYZ we should return here then,
>> or just not emit the subsequent texture instruction.'
>
> There is this check for textureSize() :
>
>       if (inst->Dst[0].Register.WriteMask & 0x7) {
>
> However, I think it's worth adding a return after this block (although
> it doesn't seem necessary)
>
>> stick a -t texture_query_levels (or whatever causes them to run).
>>
>
> They all pass here, although it make sense to add a return there, I'll send v3.

Okay I'll look at your v3, and try and fix up the fail here.

some of the geom shader tests are getting a TXQ when translating from GLSL,
which isn't correct, but shouldn't break things either.

Dave.