[Mesa-dev] [PATCH 20/75] st/nine: Initial ProcessVertices support

[Ilia Mirkin]

On Thu, Oct 6, 2016 at 4:24 PM, Axel Davy <axel.davy at ens.fr> wrote:
> On 05/10/2016 22:08, Axel Davy wrote:
>>
>>   HRESULT NINE_WINAPI
>>   NineDevice9_ProcessVertices( struct NineDevice9 *This,
>>                                UINT SrcStartIndex,
>> @@ -3174,33 +3188,69 @@ NineDevice9_ProcessVertices( struct NineDevice9
>> *This,
>>                                IDirect3DVertexDeclaration9 *pVertexDecl,
>>                                DWORD Flags )
>>   {
>> -    struct pipe_screen *screen = This->screen;
>> +    struct pipe_screen *screen_sw = This->screen_sw;
>> +    struct pipe_context *pipe_sw = This->pipe_sw;
>>       struct NineVertexDeclaration9 *vdecl =
>> NineVertexDeclaration9(pVertexDecl);
>> +    struct NineVertexBuffer9 *dst = NineVertexBuffer9(pDestBuffer);
>>       struct NineVertexShader9 *vs;
>>       struct pipe_resource *resource;
>> +    struct pipe_transfer *transfer = NULL;
>> +    struct pipe_stream_output_info so;
>>       struct pipe_stream_output_target *target;
>>       struct pipe_draw_info draw;
>> +    struct pipe_box box;
>> +    unsigned offsets[1] = {0};
>>       HRESULT hr;
>> -    unsigned buffer_offset, buffer_size;
>> +    unsigned buffer_size;
>> +    void *map;
>>         DBG("This=%p SrcStartIndex=%u DestIndex=%u VertexCount=%u "
>>           "pDestBuffer=%p pVertexDecl=%p Flags=%d\n",
>>           This, SrcStartIndex, DestIndex, VertexCount, pDestBuffer,
>>           pVertexDecl, Flags);
>>   -    if (!screen->get_param(screen, PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS))
>> -        STUB(D3DERR_INVALIDCALL);
>> +    if (!screen_sw->get_param(screen_sw,
>> PIPE_CAP_MAX_STREAM_OUTPUT_BUFFERS)) {
>> +        DBG("ProcessVertices not supported\n");
>> +        return D3DERR_INVALIDCALL;
>> +    }
>>   -    nine_update_state(This);
>>   -    /* TODO: Create shader with stream output. */
>> -    STUB(D3DERR_INVALIDCALL);
>> -    struct NineVertexBuffer9 *dst = NineVertexBuffer9(pDestBuffer);
>> +    vs = This->state.programmable_vs ? This->state.vs : This->ff.vs;
>> +    /* Note: version is 0 for ff */
>> +    user_assert(vdecl || (vs->byte_code.version < 0x30 && dst->desc.FVF),
>> +                D3DERR_INVALIDCALL);
>> +    if (!vdecl) {
>> +        DWORD FVF = dst->desc.FVF;
>> +        vdecl = util_hash_table_get(This->ff.ht_fvf, &FVF);
>> +        if (!vdecl) {
>> +            hr = NineVertexDeclaration9_new_from_fvf(This, FVF, &vdecl);
>> +            if (FAILED(hr))
>> +                return hr;
>> +            vdecl->fvf = FVF;
>> +            util_hash_table_set(This->ff.ht_fvf, &vdecl->fvf, vdecl);
>> +            NineUnknown_ConvertRefToBind(NineUnknown(vdecl));
>> +        }
>> +    }
>>   -    vs = This->state.vs ? This->state.vs : This->ff.vs;
>> +    /* Flags: Can be 0 or D3DPV_DONOTCOPYDATA, and/or lock flags
>> +     * D3DPV_DONOTCOPYDATA -> Has effect only for ff. In particular
>> +     * if not set, everything from src will be used, and dst
>> +     * must match exactly the ff vs outputs.
>> +     * TODO: Handle all the checks, etc for ff */
>> +    user_assert(vdecl->position_t || This->state.programmable_vs,
>> +                D3DERR_INVALIDCALL);
>> +
>> +    /* TODO: Support vs < 3 and ff */
>> +    user_assert(vs->byte_code.version == 0x30,
>> +                D3DERR_INVALIDCALL);
>> +    /* TODO: Not hardcode the constant buffers for swvp */
>> +    user_assert(This->may_swvp,
>> +                D3DERR_INVALIDCALL);
>> +
>> +    nine_state_prepare_draw_sw(This, vdecl, SrcStartIndex, VertexCount,
>> &so);
>>   -    buffer_size = VertexCount * vs->so->stride[0];
>> -    if (1) {
>> +    buffer_size = VertexCount * so.stride[0] * 4;
>> +    {
>>           struct pipe_resource templ;
>>             templ.target = PIPE_BUFFER;
>> @@ -3212,49 +3262,50 @@ NineDevice9_ProcessVertices( struct NineDevice9
>> *This,
>>           templ.height0 = templ.depth0 = templ.array_size = 1;
>>           templ.last_level = templ.nr_samples = 0;
>>   -        resource = This->screen->resource_create(This->screen, &templ);
>> +        resource = screen_sw->resource_create(screen_sw, &templ);
>>           if (!resource)
>>               return E_OUTOFMEMORY;
>> -        buffer_offset = 0;
>> -    } else {
>> -        /* SO matches vertex declaration */
>> -        resource = NineVertexBuffer9_GetResource(dst);
>> -        buffer_offset = DestIndex * vs->so->stride[0];
>>       }
>> -    target = This->pipe->create_stream_output_target(This->pipe,
>> resource,
>> -                                                     buffer_offset,
>> -                                                     buffer_size);
>> +    target = pipe_sw->create_stream_output_target(pipe_sw, resource,
>> +                                                  0, buffer_size);
>>       if (!target) {
>>           pipe_resource_reference(&resource, NULL);
>>           return D3DERR_DRIVERINTERNALERROR;
>>       }
>>   -    if (!vdecl) {
>> -        hr = NineVertexDeclaration9_new_from_fvf(This, dst->desc.FVF,
>> &vdecl);
>> -        if (FAILED(hr))
>> -            goto out;
>> -    }
>> -
>>       init_draw_info(&draw, This, D3DPT_POINTLIST, VertexCount);
>>       draw.instance_count = 1;
>>       draw.indexed = FALSE;
>> -    draw.start = SrcStartIndex;
>> +    draw.start = 0;
>>       draw.index_bias = 0;
>> -    draw.min_index = SrcStartIndex;
>> -    draw.max_index = SrcStartIndex + VertexCount - 1;
>> +    draw.min_index = 0;
>> +    draw.max_index = VertexCount - 1;
>> +
>> +
>> +    pipe_sw->set_stream_output_targets(pipe_sw, 1, &target, offsets);
>>   -    This->pipe->set_stream_output_targets(This->pipe, 1, &target, 0);
>> -    This->pipe->draw_vbo(This->pipe, &draw);
>> -    This->pipe->set_stream_output_targets(This->pipe, 0, NULL, 0);
>> -    This->pipe->stream_output_target_destroy(This->pipe, target);
>> +    pipe_sw->draw_vbo(pipe_sw, &draw);
>> +
>> +    pipe_sw->set_stream_output_targets(pipe_sw, 0, NULL, 0);
>> +    pipe_sw->stream_output_target_destroy(pipe_sw, target);
>> +
>> +    u_box_1d(0, VertexCount * so.stride[0] * 4, &box);
>> +    map = pipe_sw->transfer_map(pipe_sw, resource, 0, PIPE_TRANSFER_READ,
>> &box,
>> +                                &transfer);
>> +    if (!map) {
>> +        hr = D3DERR_DRIVERINTERNALERROR;
>> +        goto out;
>> +    }
>>         hr = NineVertexDeclaration9_ConvertStreamOutput(vdecl,
>>                                                       dst, DestIndex,
>> VertexCount,
>> -                                                    resource, vs->so);
>> +                                                    map, &so);
>> +    if (transfer)
>> +        pipe_sw->transfer_unmap(pipe_sw, transfer);
>> +
>>   out:
>> +    nine_state_after_draw_sw(This);
>>       pipe_resource_reference(&resource, NULL);
>> -    if (!pVertexDecl)
>> -        NineUnknown_Release(NineUnknown(vdecl));
>>       return hr;
>>   }
>>
>
> Hi,
>
> For some reason, this is not working well anymore on both llvmpipe and
> softpipe (but If I force screen_sw to take hal, that is radeonsi, the
> function works).
>
> It crashes at the last
>
> pipe_resource_reference(&resource, NULL);
>
>
> However the resource refcount is 1, so I don't see the problem....
> The patch used to work, but perhaps I broke it when modifying a few other
> stuffs.
> Any idea why this would crash ?

Recently a "next" pointer has been added to pipe_resource which has
been causing a few people grief as it's not always nulled out in the
initial templates. Not sure if it's relevant to this case or not.

  -ilia