Re: [1/5] WineD3D: Get some vertex pipeline infrastructure in place

[Henri Verbeet]

2008/11/17 Stefan Dösinger :
> - } else if (fixup ||
> - } else if (device->vertex_pipe->can_convert_d3dcolor || fixup ||

Shouldn't this be redundant if you already handle color conversion in
IWineD3DVertexBufferImpl_FindDecl()?
> - } else {
> - } else if(!device->vertex_pipe->can_convert_d3dcolor) {

I don't think this is how we want to do this. I think it makes more
sense to ask the pipeline if it can handle a particular type for a
particular usage. You can handle FLOAT16 support in the same way then.
Also note that EXT_vertex_array_bgra adds support for D3DCOLOR, but
only for diffuse, specular and generic attributes. That should be
enough for anything we come across, but with a "can_convert_d3dcolor"
flag like that you can't handle that extension properly.

[1/5] WineD3D: Get some vertex pipeline infrastructure in place

[Stefan Dösinger]

> 2008/11/17 Stefan Dösinger :
> > - } else if (fixup ||
> > - } else if (device->vertex_pipe->can_convert_d3dcolor || fixup ||
>
> Shouldn't this be redundant if you already handle color conversion in
> IWineD3DVertexBufferImpl_FindDecl()?

Not all vertex data comes from a vertex buffer. 'fixup' is always FALSE for Draw[Indexed]PrimitiveUP and DrawPrimitiveStrided.
> > - } else {
> > - } else if(!device->vertex_pipe->can_convert_d3dcolor) {
>
> I don't think this is how we want to do this. I think it makes more
> sense to ask the pipeline if it can handle a particular type for a
> particular usage. You can handle FLOAT16 support in the same way then.

Good point, although in practise we don't have to be able to handle all types with all usages. On some windows drivers e.g. FLOAT16 colors and fixed function fail, and ati returns an error. I think we can make use of that to keep the code simpler.
> Also note that EXT_vertex_array_bgra adds support for D3DCOLOR, but
> only for diffuse, specular and generic attributes. That should be
> enough for anything we come across, but with a "can_convert_d3dcolor"
> flag like that you can't handle that extension properly.

Hmm, I think I should make this a function instead of a flag to deal with EXT_vertex_array_bgra

So better don't apply this patch for now

[1/5] WineD3D: Get some vertex pipeline infrastructure in place

[Henri Verbeet]

2008/11/17 Stefan Dösinger :
> > 2008/11/17 Stefan Dösinger :
> > > - } else if (fixup ||
> > > - } else if (device->vertex_pipe->can_convert_d3dcolor || fixup ||
> >
> > Shouldn't this be redundant if you already handle color conversion in
> > IWineD3DVertexBufferImpl_FindDecl()?
>
> Not all vertex data comes from a vertex buffer. 'fixup' is always FALSE for Draw[Indexed]PrimitiveUP and DrawPrimitiveStrided.

Can we fix that? I think I would prefer it if
primitiveDeclarationConvertToStridedData() handled all that, and just
returned whether we can use the fast draw function, or have to use the
slow one.

[1/5] WineD3D: Get some vertex pipeline infrastructure in place

[Stefan Dösinger]

> Can we fix that? I think I would prefer it if
> primitiveDeclarationConvertToStridedData() handled all that, and just
> returned whether we can use the fast draw function, or have to use the
> slow one.

We never call primitiveDeclarationConverToStridedData() with drawPrimitiveStrided. We could of course add a function that checks the application provided strided data(ie, the very if statement we have here), but I don't think that gains us a lot