Re: [PATCH 2/5] wined3d: Implement copying of buffer resources.

[Henri Verbeet]

On 18 March 2016 at 12:30, Józef Kucia <jkucia(a)codeweavers.com> wrote:
> +HRESULT wined3d_buffer_copy(struct wined3d_buffer *dst_buffer, unsigned int dst_offset,
> +        struct wined3d_buffer *src_buffer, unsigned int src_offset, unsigned int size)
> +{
> +    BYTE *dst_buffer_mem, *src_buffer_mem;
> +    const struct wined3d_gl_info *gl_info;
> +    struct wined3d_context *context;
> +    struct wined3d_device *device;
> +
> +    device = dst_buffer->resource.device;
> +
> +    context = context_acquire(device, NULL);
> +    gl_info = context->gl_info;
> +
> +    buffer_internal_preload(dst_buffer, context, NULL);
> +    buffer_internal_preload(src_buffer, context, NULL);
> +
> +    dst_buffer_mem = dst_buffer->resource.heap_memory;
> +    src_buffer_mem = src_buffer->resource.heap_memory;
> This is probably fine, but I wonder why you're not using
> buffer_get_memory()? Somewhat related, there may be something to be
> said for the function just taking wined3d_bo_address structures and a
> size, although I don't have an immediate use for that yet.
> +
> +    if (!dst_buffer_mem && !src_buffer_mem)
> +    {
> +        if (!gl_info->supported[ARB_COPY_BUFFER])
> +        {
> +            FIXME("Direct buffer copy not supported.\n");
> +            return WINED3DERR_INVALIDCALL;
> +        }
> I think that's fine, but we could of course implement that case by
> mapping the buffers and using memcpy().

[PATCH 2/5] wined3d: Implement copying of buffer resources.

[Józef Kucia]

On Sat, Mar 19, 2016 at 10:15 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> On 18 March 2016 at 12:30, Józef Kucia <jkucia(a)codeweavers.com> wrote:
> > +HRESULT wined3d_buffer_copy(struct wined3d_buffer *dst_buffer, unsigned int dst_offset,
> > +        struct wined3d_buffer *src_buffer, unsigned int src_offset, unsigned int size)
> > +{
> > +    BYTE *dst_buffer_mem, *src_buffer_mem;
> > +    const struct wined3d_gl_info *gl_info;
> > +    struct wined3d_context *context;
> > +    struct wined3d_device *device;
> > +
> > +    device = dst_buffer->resource.device;
> > +
> > +    context = context_acquire(device, NULL);
> > +    gl_info = context->gl_info;
> > +
> > +    buffer_internal_preload(dst_buffer, context, NULL);
> > +    buffer_internal_preload(src_buffer, context, NULL);
> > +
> > +    dst_buffer_mem = dst_buffer->resource.heap_memory;
> > +    src_buffer_mem = src_buffer->resource.heap_memory;
> > This is probably fine, but I wonder why you're not using
> > buffer_get_memory()? Somewhat related, there may be something to be
> > said for the function just taking wined3d_bo_address structures and a
> > size, although I don't have an immediate use for that yet.

You're right, buffer_get_memory() should be better in this case.
> > +
> > +    if (!dst_buffer_mem && !src_buffer_mem)
> > +    {
> > +        if (!gl_info->supported[ARB_COPY_BUFFER])
> > +        {
> > +            FIXME("Direct buffer copy not supported.\n");
> > +            return WINED3DERR_INVALIDCALL;
> > +        }
> > I think that's fine, but we could of course implement that case by
> > mapping the buffers and using memcpy().

It can be implemented. However, I don't expect it to be very useful in practice.

[PATCH 2/5] wined3d: Implement copying of buffer resources.

[Henri Verbeet]

On 19 March 2016 at 10:46, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
> It can be implemented. However, I don't expect it to be very useful in practice.
> Yeah. What we tend to do is to just put in a small comment that
> describes how it could be implemented and why it isn't.