[PATCH 0/1] MR4991: d3d10/effect: Clarify constant buffer flags field meaning.

[Nikolay Sivov (＠nsivov)]

It makes more sense to have it matching fx_5_0 format, which
uses the same flag for TBUFFER case, and also keeps 'single' modifier in there.
This modifier is ignored on fx_4_x, so it's only needed to be implemented
in the compiler.

Signed-off-by: Nikolay Sivov

[PATCH 1/1] d3d10/effect: Clarify constant buffer flags field meaning.

[Nikolay Sivov]

From: Nikolay Sivov

It makes more sense to have it matching fx_5_0 format, which
uses the same flag for TBUFFER case, and also keeps 'single' modifier in there.
This modifier is ignored on fx_4_x, so it's only needed to be implemented
in the compiler.

Signed-off-by: Nikolay Sivov 
---
 dlls/d3d10/effect.c | 51 +++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c
index 534eadb2659..26394510945 100644
--- a/dlls/d3d10/effect.c
+++ b/dlls/d3d10/effect.c
@@ -3796,10 +3796,13 @@ static HRESULT create_buffer_object(struct d3d10_effect_variable *v)
 static HRESULT parse_fx10_buffer(const char *data, size_t data_size, const char **ptr,
         BOOL local, struct d3d10_effect_variable *l)
 {
+    enum buffer_flags
+    {
+        IS_TBUFFER = 1,
+    };
     const char *prefix = local ? "Local" : "Shared";
+    uint32_t offset, flags;
     unsigned int i;
-    uint32_t offset;
-    D3D10_CBUFFER_TYPE d3d10_cbuffer_type;
     HRESULT hr;
     unsigned int stride = 0;

@@ -3826,32 +3829,30 @@ static HRESULT parse_fx10_buffer(const char *data, size_t data_size, const char
     l->data_size = read_u32(ptr);
     TRACE("%s buffer data size: %#x.\n", prefix, l->data_size);

-    d3d10_cbuffer_type = read_u32(ptr);
-    TRACE("%s buffer type: %#x.\n", prefix, d3d10_cbuffer_type);
+    flags = read_u32(ptr);
+    TRACE("%s buffer flags: %#x.\n", prefix, flags);

-    switch(d3d10_cbuffer_type)
+    if (flags & IS_TBUFFER)
     {
-        case D3D10_CT_CBUFFER:
-            l->type->basetype = D3D10_SVT_CBUFFER;
-            if (!copy_name("cbuffer", &l->type->name))
-            {
-                ERR("Failed to copy name.\n");
-                return E_OUTOFMEMORY;
-            }
-            break;
-
-        case D3D10_CT_TBUFFER:
-            l->type->basetype = D3D10_SVT_TBUFFER;
-            if (!copy_name("tbuffer", &l->type->name))
-            {
-                ERR("Failed to copy name.\n");
-                return E_OUTOFMEMORY;
-            }
-            break;
+        l->type->basetype = D3D10_SVT_TBUFFER;
+        copy_name("tbuffer", &l->type->name);
+    }
+    else
+    {
+        l->type->basetype = D3D10_SVT_CBUFFER;
+        copy_name("cbuffer", &l->type->name);
+    }
+    if (!l->type->name)
+    {
+        ERR("Failed to copy name.\n");
+        return E_OUTOFMEMORY;
+    }

-        default:
-            ERR("Unexpected D3D10_CBUFFER_TYPE %#x!\n", d3d10_cbuffer_type);
-            return E_FAIL;
+    flags &= ~IS_TBUFFER;
+    if (flags)
+    {
+        ERR("Unexpected buffer flags %#x.\n", flags);
+        return E_FAIL;
     }

l->type->member_count = read_u32(ptr);

[Matteo Bruni (＠Mystral)]

My only question with this is whether the old code simply abused D3D10_CBUFFER_TYPE. I.e., do D3D_CT_INTERFACE_POINTERS and D3D_CT_RESOURCE_BIND_INFO have nothing to do with this field in fx_5_0 binary effects? Can you get those values with newer effect formats?

That's only a concern for using the local enum vs keeping D3D10_CBUFFER_TYPE. I have no issues with the rest of the patch either way.

[Nikolay Sivov (＠nsivov)]

On Thu Feb  1 21:08:23 2024 +0000, Matteo Bruni wrote:
> My only question with this is whether the old code simply abused
> D3D10_CBUFFER_TYPE. I.e., do D3D_CT_INTERFACE_POINTERS and
> D3D_CT_RESOURCE_BIND_INFO have nothing to do with this field in fx_5_0
> binary effects? Can you get those values with newer effect formats?
> That's only a concern for using the local enum vs keeping
> D3D10_CBUFFER_TYPE. I have no issues with the rest of the patch either way.

Yes, I can see why it was picked because it seems related, but this field is not D3D_CBUFFER_TYPE. Simple evidence is that 'single' modifier that is supported on fx_5_0 is encoded as next bit 0x2 in this field. And that's D3D_CT_INTERFACE_POINTERS value.

[PATCH 0/1] MR4991: d3d10/effect: Clarify constant buffer flags field meaning. - approved

[Matteo Bruni (＠Mystral)]

This merge request was approved by Matteo Bruni.