New front face culling flag

[vitek]

The new patch is using cull mode wrong, at least on D3D. This is what MSDN has to say about D3DRS_CULLMODE...

Code: Select all

D3DRS_CULLMODE
Specifies how back-facing triangles are culled, if at all. This can be set to one member of the D3DCULL enumerated type. The default value is D3DCULL_CCW. 

This is how we are using it...

Code: Select all

if (resetAllRenderstates || (lastmaterial.FrontfaceCulling != material.FrontfaceCulling) || (lastmaterial.BackfaceCulling != material.BackfaceCulling))
{
   if (material.FrontfaceCulling && material.BackfaceCulling)
      pID3DDevice->SetRenderState(D3DRS_CULLMODE, D3DCULL_CW|D3DCULL_CCW);
   else
   if (material.FrontfaceCulling)
      pID3DDevice->SetRenderState(D3DRS_CULLMODE, D3DCULL_CW);
   else
   if (material.BackfaceCulling)
      pID3DDevice->SetRenderState(D3DRS_CULLMODE, D3DCULL_CCW);
   else
      pID3DDevice->SetRenderState(D3DRS_CULLMODE, D3DCULL_NONE);

I'm not sure what the expected behavior is if both BackfaceCulling and FrontfaceCulling are set. I'm guessing that this is intended to be compatible with OpenGL, so lines and points would still be rendered. In that case, some code would need to be added to CD3D?Driver.cpp to prevent it from drawing triangle strip/fan/lists when both cull flags are set. At least I think that is what should be happening.

Honestly, I would have expected BackcaceCulling to be changed to CullMode and three seperate values added. This is compatible with both OpenGL and D3D, and can be easily imlemented in the software driver for consistency.

Travis

[hybrid]

So this means that the Or o fthe values is not allowed? That's a pity. Unnecessarily complex workarounds, but that's the price for cross-platforms
I don't see why using a cullmode enum would be any simpler, or different at all. It would just use 2 bytes more.

[vitek]

So this means that the Or o fthe values is not allowed? That's a pity.

Yup.

I don't see why using a cullmode enum would be any simpler, or different at all. It would just use 2 bytes more.

I don't understand how you would opt to use 2 more bytes. I'm guessing that you're saying you would change the value to a full 32-bit quantity, but that doesn't really make any sense given that there are only three posible values. This could easily be reduced to just one u8 which is likely to be half as large as the two bools that are currently in place.

The advantage of using an enum is that you control the allowed options. This makes it quite easy to prevent the user from enabling both CCW and CW culling on D3D8/9, while still providing access to the culling modes supported consistently by all drivers.

Also, it shouldn't be to difficult to add cull mode support to the software drivers. Hint, hint...

Travis

[alexionne]

Just an observation: material which have turned on both front and back culling is actually invisible. Maybe this fact could be used in DX (or even GL and other) implementation?

[hybrid]

OpenGL does provide all four culling modes, which is why I chose this way. The D3D implementation was suggested on IRC. I'll add a check to the draw method in order to prevent poly drawing if both sides are culled.
I tend to ignore the software drivers, sorry about that. I hope that someone who uses one of them for something will improve them in turn. Until then you shouldn't expect to get too many new featues there.

[BlindSide]

Sorry hybrid I suggested that method because thats what they did in this patch, I didn't know that it was not possible. The enum sounds like a nice idea, that, or just default to frontface culling if both are selected.