Re: [PATCH 3/6] wined3d: Set an array of stream sources as a single CS operation.

[Henri Verbeet]

On Thu, 8 Jul 2021 at 01:13, Zebediah Figura  wrote:
> @@ -841,26 +841,26 @@ static void STDMETHODCALLTYPE d3d11_device_context_IASetInputLayout(ID3D11Device
>  static void STDMETHODCALLTYPE d3d11_device_context_IASetVertexBuffers(ID3D11DeviceContext1 *iface,
>          UINT start_slot, UINT buffer_count, ID3D11Buffer *const *buffers, const UINT *strides, const UINT *offsets)
>  {
> -
>
> struct wined3d_stream_state streams[D3D11_IA_VERTEX_INPUT_RESOURCE_SLOT_COUNT];
> struct d3d11_device_context *context = impl_from_ID3D11DeviceContext1(iface);
> unsigned int i;
>
> TRACE("iface %p, start_slot %u, buffer_count %u, buffers %p, strides %p, offsets %p.\n",
>         iface, start_slot, buffer_count, buffers, strides, offsets);
> -
>
> wined3d_mutex_lock();
> for (i = 0; i < buffer_count; ++i)
> {
>     struct d3d_buffer *buffer = unsafe_impl_from_ID3D11Buffer(buffers[i]);
> -
>    struct wined3d_stream_state stream;
>
> -
>    stream.buffer = buffer ? buffer->wined3d_buffer : NULL;
> -
>    stream.offset = offsets[i];
> -
>    stream.stride = strides[i];
> -
>    stream.frequency = 1;
> -
>    stream.flags = 0;
> -
> -
>    wined3d_device_context_set_stream_source(context->wined3d_context, start_slot + i, &stream);
> -
>    streams[i].buffer = buffer ? buffer->wined3d_buffer : NULL;
> -
>    streams[i].offset = offsets[i];
> -
>    streams[i].stride = strides[i];
> -
>    streams[i].frequency = 1;
> -
>    streams[i].flags = 0;
> }
> -
> - wined3d_mutex_lock();
> - wined3d_device_context_set_stream_sources(context->wined3d_context, start_slot, buffer_count, streams);
> wined3d_mutex_unlock();
>
> }

Somewhat like the stream output patch, this changes behaviour; where
previously we'd ignore invalid stream indices and print a WARN, we'd
now write past the end of the streams[] buffer.
> -HRESULT CDECL wined3d_device_context_set_stream_source(struct wined3d_device_context *context,
> -
>    unsigned int stream_idx, const struct wined3d_stream_state *src_stream)
>
> +HRESULT CDECL wined3d_device_context_set_stream_sources(struct wined3d_device_context *context,
> -
>    unsigned int start_idx, unsigned int count, const struct wined3d_stream_state *streams)
>
> {
> - struct wined3d_buffer *buffer = src_stream->buffer;
> - struct wined3d_stream_state *dst_stream;
> - struct wined3d_buffer *prev_buffer;
> - struct wined3d_state *state = context->state;
> - unsigned int i;
> - TRACE("context %p, stream_idx %u, src_stream %p.\n", context, stream_idx, src_stream);
> - TRACE("context %p, start_idx %u, count %u, streams %p.\n", context, start_idx, count, streams);
> - if (stream_idx >= WINED3D_MAX_STREAMS)
> - if (!wined3d_bound_range(start_idx, count, WINED3D_MAX_STREAMS))
> {
> -
>    WARN("Stream index %u out of range.\n", stream_idx);
> -
>    return WINED3DERR_INVALIDCALL;
> - }
> - else if (src_stream->offset & 0x3)
> - {
> -
>    WARN("Offset %u is not 4 byte aligned.\n", src_stream->offset);
> -
>    WARN("Start index %u is out of range.\n", start_idx);
>    return WINED3DERR_INVALIDCALL;
> }
> - dst_stream = &context->state->streams[stream_idx];
> - prev_buffer = dst_stream->buffer;
> - count = min(count, WINED3D_MAX_STREAMS - start_idx);
> - if (!memcmp(src_stream, dst_stream, sizeof(*src_stream)))
> - for (i = 0; i < count; ++i)
> {
> -
>    TRACE("Application is setting the old values over, nothing to do.\n");
> -
>    if (streams[i].offset & 0x3)
> -
>    {
> -
>        WARN("Offset %u is not 4 byte aligned.\n", streams[i].offset);
> -
>        return WINED3DERR_INVALIDCALL;
> -
>    }
> - }
> -
> - if (!memcmp(streams, &state->streams[start_idx], count * sizeof(*streams)))
>     return WINED3D_OK;
> -
>
> }
> -
>
> *dst_stream = *src_stream;
> -
>
> if (buffer)
> -
>    wined3d_buffer_incref(buffer);
> -
>
> wined3d_device_context_emit_set_stream_source(context, stream_idx, src_stream);
> -
>
> if (prev_buffer)
> -
>    wined3d_buffer_decref(prev_buffer);
> -
>
> wined3d_device_context_emit_set_stream_sources(context, start_idx, count, streams);
> -
>
> for (i = 0; i < count; ++i)
> -
>
> {
> -
>    struct wined3d_buffer *prev = state->streams[start_idx + i].buffer;
> -
>    struct wined3d_buffer *buffer = streams[i].buffer;
> -
> -
>    state->streams[start_idx + i] = streams[i];
> -
> -
>    if (buffer)
> -
>        wined3d_buffer_incref(buffer);
> -
>    if (prev)
> -
>        wined3d_buffer_decref(prev);
> -
>
> }
>
> return WINED3D_OK;
>
> }

And here we end up rejecting the entire call if any of the streams are
invalid. That seems entirely reasonable, but I'd still feel better
about it if it was a separate change.