Re: [PATCH] d3dx9: Move object initialization into a separate function.
2013/10/9 Rico Schüller kgbric...@web.de: On 09.10.2013 01:12, Matteo Bruni wrote: Hi Rico, 2013/10/8 Rico Schüller kgbric...@web.de: Hi, this moves the object initialization into a separate function, so it could be used for strings and resources. It also removes the STATE_TYPE as we could distinguish the types at the object level. 1. When an object has a destination, it points to another shader variable. This was state ST_PARAMETER. 2. If a variable has something in data, it is fxlc, shader (preshader) or a string. This was state ST_CONSTANT and ST_FXLC. 3. If it has both (destination and data), it points to an array of shader variables. The name is in the destination, the index could be calculated with the data. This will be added in a later patch. There's still the issue of distinguishing between ST_CONSTANT and ST_FXLC, checking object_id and type might cover that though. I think we could distinguish between them. I forgot to add, if both are 0, then it is a constant and the parameter data has already the correct value. Marking the shader as ST_CONSTANT was a little bit wrong, as we would need to set the shader/preshader variables. Also saving the destination parameter in the object gains some speed when we need to access the variable as we don't need to run get_parameter_by_name() each time we need the variable ... I'm not sure storing additional info into the objects is the right thing to do. Take a look at the D3DXFX_NOT_CLONEABLE flag from http://msdn.microsoft.com/en-us/library/windows/desktop/bb172855%28v=vs.85%29.aspx. Notice that GetPassDesc() doesn't return the shader data if the object was created with the flag. What I've been thinking is that it simply can't because the original shader data, stored in an object, were freed after parsing. Yeah, I'm aware of D3DXFX_NOT_CLONEABLE. But we need to hold the shader blob or something similar (a reflection of the used preshader variables) to set the correct values. We also need it in the case for when the parameter needs to be calculated (fxlc) and in for strings. So imho, we could only release the blob partially when we have a preshader, nowhere else (and in this case we still need the reflection somehow). So let me conclude: We need to store the destination and the reflection somewhere. We may release the full shader binary. Yeah, we need to store something for those cases, but not necessarily the original shader blob (we could store some derived information instead). So I essentially agree here. While nothing forces us to do the same (except probably avoiding to use more memory than strictly necessary) I think it's better not to put additional stuff into the objects or generally assume that the objects are still available after parsing. That means creating the shaders and the strings at parse time or right after that and storing any additional required information (e.g. preshader) in the parameter. So, directly storing the referenced parameter is a good idea but I'd prefer that pointer to be in d3dx_parameter. I haven't put it in the parameter as there are much more parameters than objects. Each state, each array element and each structure member has a parameter while there are only some parameters that have objects. So we may use some more bytes when putting it in the parameter than putting it in the object. True, my point is that the memory reclaimed by freeing the objects is probably more than the memory wasted by some additional pointers. I don't have any hard data though (and applications might forget to specify D3DXFX_NOT_CLONEABLE) so I might be wrong. I'm fine with both ways, because each object is tight to a specific parameter, it's mostly a matter of taste where the data is stored. Cheers Rico
Re: [PATCH] d3dx9: Move object initialization into a separate function.
Hi Rico, 2013/10/8 Rico Schüller kgbric...@web.de: Hi, this moves the object initialization into a separate function, so it could be used for strings and resources. It also removes the STATE_TYPE as we could distinguish the types at the object level. 1. When an object has a destination, it points to another shader variable. This was state ST_PARAMETER. 2. If a variable has something in data, it is fxlc, shader (preshader) or a string. This was state ST_CONSTANT and ST_FXLC. 3. If it has both (destination and data), it points to an array of shader variables. The name is in the destination, the index could be calculated with the data. This will be added in a later patch. There's still the issue of distinguishing between ST_CONSTANT and ST_FXLC, checking object_id and type might cover that though. Also saving the destination parameter in the object gains some speed when we need to access the variable as we don't need to run get_parameter_by_name() each time we need the variable ... I'm not sure storing additional info into the objects is the right thing to do. Take a look at the D3DXFX_NOT_CLONEABLE flag from http://msdn.microsoft.com/en-us/library/windows/desktop/bb172855%28v=vs.85%29.aspx. Notice that GetPassDesc() doesn't return the shader data if the object was created with the flag. What I've been thinking is that it simply can't because the original shader data, stored in an object, were freed after parsing. While nothing forces us to do the same (except probably avoiding to use more memory than strictly necessary) I think it's better not to put additional stuff into the objects or generally assume that the objects are still available after parsing. That means creating the shaders and the strings at parse time or right after that and storing any additional required information (e.g. preshader) in the parameter. So, directly storing the referenced parameter is a good idea but I'd prefer that pointer to be in d3dx_parameter. Cheers Rico --- dlls/d3dx9_36/effect.c | 98 ++ 1 Datei geändert, 34 Zeilen hinzugefügt(+), 64 Zeilen entfernt(-) Cheers, Matteo
Re: [PATCH] d3dx9: Use struct d3dx_object for objects.
2013/9/24 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/effect.c | 248 +++-- 1 Datei geändert, 95 Zeilen hinzugefügt(+), 153 Zeilen entfernt(-) I definitely like the direction this patch takes. @@ -5068,6 +5009,9 @@ static HRESULT d3dx9_parse_resource(struct d3dx9_base_effect *base, const char * param = state-parameter; +/* + * TODO: Do we need to create the shader/string here or later when we access them? + */ Here it's fine AFAICS. What are the other options you are thinking about? Something like creating the shaders at the first Begin / BeginPass call?
Re: [PATCH] wined3d: Fix per-stage constant in GLSL fixed function pipeline + add tests. (try 2)
2013/9/4 Christian Costa titan.co...@gmail.com:
This patch fixes part of bug 33606 and 31918.
The current code generates shaders that support per-stage constant but does
not declare the variables used causing thus compilation failures.
With this patch, these variables are declared and updated when needed. Tests
are also added.
Thanks to Matteo Bruni for review and support.
Try 2:
- update with latest git
Hi, I have a few small nits still, please bear with me...
---
dlls/d3d9/tests/visual.c | 80
dlls/wined3d/glsl_shader.c | 34 ++-
2 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c
index b856c93..6072c23 100644
--- a/dlls/d3d9/tests/visual.c
+++ b/dlls/d3d9/tests/visual.c
@@ -10742,6 +10742,85 @@ static void dp3_alpha_test(IDirect3DDevice9 *device)
{
ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
}
+static void perstage_constant_test(IDirect3DDevice9 *device)
+{
+HRESULT hr;
+D3DCAPS9 caps;
+DWORD color;
+struct vertex quad[] = {
+{ -1.0,-1.0,0.1,0x408080c0 },
+{ 1.0,-1.0,0.1,0x408080c0 },
+{ -1.0, 1.0,0.1,0x408080c0 },
+{ 1.0, 1.0,0.1,0x408080c0 },
+};
+
Can you please make quad[] static const? Also, add the 'f' suffix to
the float constants and move the opening brace to newline.
+memset(caps, 0, sizeof(caps));
+hr = IDirect3DDevice9_GetDeviceCaps(device, caps);
+ok(SUCCEEDED(hr), GetDeviceCaps failed with 0x%08x\n, hr);
+if (!(caps.PrimitiveMiscCaps D3DPMISCCAPS_PERSTAGECONSTANT))
+{
+skip(D3DPMISCCAPS_PERSTAGECONSTANT not supported\n);
Please put a '.' at the end of the message. Same for the following prints.
+return;
+}
+
+hr = IDirect3DDevice9_SetFVF(device, D3DFVF_XYZ | D3DFVF_DIFFUSE);
+ok(hr == D3D_OK, IDirect3DDevice9_SetFVF failed with 0x%08x\n, hr);
+
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_CONSTANT,
0x80a1b2c3);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+
+/* Check color values */
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP,
D3DTOP_SELECTARG1);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1,
D3DTA_CONSTANT);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+
Although you don't strictly need it here, I'd explicitly set
D3DTSS_COLOROP for stage 1 to D3DTOP_DISABLE. Also maybe set
D3DTSS_ALPHAOP for stage 0 to disabled?
+hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET |
D3DCLEAR_ZBUFFER, 0x, 1.0f, 0);
+ok(hr == D3D_OK, IDirect3DDevice9_Clear failed with 0x%08x\n, hr);
+
+hr = IDirect3DDevice9_BeginScene(device);
+ok(hr == D3D_OK, IDirect3DDevice9_BeginScene failed with 0x%08x\n, hr);
+hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2,
quad, sizeof(*quad));
+ok(SUCCEEDED(hr), DrawPrimitiveUP failed with 0x%08x\n, hr);
+hr = IDirect3DDevice9_EndScene(device);
+ok(hr == D3D_OK, IDirect3DDevice9_EndScene failed with 0x%08x\n, hr);
+
+color = getPixelColor(device, 320, 240);
+ok(color_match(color, 0x00a1b2c3, 4), perstage constant test 0x%08x,
expected 0x00a1b2c3\n, color);
+hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);
+ok(SUCCEEDED(hr), IDirect3DDevice9_Present failed with 0x%08x\n, hr);
+
+/* Check alpha value */
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLOROP,
D3DTOP_SELECTARG1);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_COLORARG1,
D3DTA_CONSTANT | D3DTA_ALPHAREPLICATE);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAOP,
D3DTOP_SELECTARG1);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+hr = IDirect3DDevice9_SetTextureStageState(device, 0, D3DTSS_ALPHAARG1,
D3DTA_CONSTANT);
+ok(hr == D3D_OK, IDirect3DDevice9_SetTextureStageState failed with
0x%08x\n, hr);
+
+hr = IDirect3DDevice9_Clear(device, 0, NULL, D3DCLEAR_TARGET |
D3DCLEAR_ZBUFFER, 0x, 1.0f, 0);
+ok(hr == D3D_OK, IDirect3DDevice9_Clear failed with 0x%08x\n, hr);
+
+hr = IDirect3DDevice9_BeginScene(device);
+ok(hr == D3D_OK, IDirect3DDevice9_BeginScene failed with 0x%08x\n, hr);
+hr = IDirect3DDevice9_DrawPrimitiveUP(device, D3DPT_TRIANGLESTRIP, 2,
quad, sizeof(*quad));
+ok(SUCCEEDED(hr
Re: [PATCH 1/5] d3d8/tests: Add cnd instruction test.
2013/9/3 Henri Verbeet hverb...@gmail.com:
On 2 September 2013 23:04, Matteo Bruni mbr...@codeweavers.com wrote:
+float quad1[] = {
+-1.0, -1.0, 0.1, 0.0,0.0,1.0,
+ 0.0, -1.0, 0.1, 1.0,0.0,1.0,
+-1.0,0.0, 0.1, 0.0,1.0,0.0,
+ 0.0,0.0, 0.1, 1.0,1.0,0.0
+};
I think that should be static const, and could do with an 'f' suffix
on the literals.
+ok(hr == D3D_OK, IDirect3DDevice8_BeginScene returned %08x\n, hr);
+if(SUCCEEDED(hr))
+{
I think the if block is pointless.
+/* 1.1 shader. All 3 components get set, based on the .w comparison */
+color = getPixelColor(device, 158, 358);
+ok(color == 0x00ff, pixel 158, 358 has color %08x, expected
0x00ff\n, color);
+color = getPixelColor(device, 162, 358);
+ok( (((color 0x00ff) 16) = 0x01) (((color 0xff00)
8) = 0x01) ((color 0x00ff) = 0x01),
+pixel 162, 358 has color %08x, expected 0x\n, color);
I think that should use color_match().
I realize those are all present in the original test, but that's more
of a reason to fix the original instead of copying it.
Totally fair points. I trusted my cleanup of the d3d9 test way too
much (also rereading the patch with some more attention would have
probably helped).
Resent, hopefully I caught all the issues. I'll cleanup the d3d9 test
as a followup.
Re: [PATCH 2/7] wined3d: Use PBOs for dynamic volumes (try 2)
2013/8/26 Stefan Dösinger ste...@codeweavers.com:
Try 2:
*) Require only GPU access for buffers
*) Reduce number of checkGLcall invocations
*) Remove a return from an ERR case
---
dlls/wined3d/utils.c | 1 +
dlls/wined3d/volume.c | 168
-
dlls/wined3d/wined3d_private.h | 7 +-
3 files changed, 157 insertions(+), 19 deletions(-)
+case WINED3D_LOCATION_BUFFER:
+if (!volume-pbo || !(volume-flags WINED3D_VFLAG_PBO))
+ERR(Trying to load WINED3D_LOCATION_BUFFER without setting
it up first.\n);
+
+if (volume-locations WINED3D_LOCATION_DISCARDED)
+{
+TRACE(Volume previously discarded, nothing to do.\n);
+wined3d_volume_invalidate_location(volume,
WINED3D_LOCATION_DISCARDED);
+}
+else if (volume-locations WINED3D_LOCATION_TEXTURE_RGB)
+{
+struct wined3d_bo_address data = {volume-pbo, NULL};
+volume_bind_and_dirtify(volume, context);
+wined3d_volume_download_data(volume, context, data);
+}
+else
+{
+FIXME(Implement WINED3D_LOCATION_SYSMEM loading from %s.\n,
+wined3d_debug_location(volume-locations));
+return;
+}
+wined3d_volume_validate_location(volume,
WINED3D_LOCATION_BUFFER);
+break;
+
Not really an actual review, but that FIXME looks wrong (copy-paste
remnant probably).
Re: [PATCH 4/5] d3dx9/tests: Add ID3DXConstantTable struct test.
2013/8/1 Matteo Bruni matteo.myst...@gmail.com: Instead of generating an entry for the struct with the correct members, the compiler generates TWO entries for sbnf, one with all its fields in D3DXRS_FLOAT4 and the other with D3DXRS_BOOL. Which, if I'm reading this correctly, makes 0 sense. Calling GetConstantByName() on the various fields then happen to return the first instance of the struct. It's the same with native d3dx9, FWIW. I take this partially back, actually it might make sense after all. Calling GetConstantDesc in general returns an array of D3DXCONSTANT_DESC, not necessarily just one. I always wondered why but I failed to make the connection to this case, until now. Long wall of text follows, sorry. Case in point: // Registers: // // Name Reg Size // - // sb b0 3 // snb b3 3 // sbn b6 2 // sbnf b8 1 // sn c0 3 // sbn c3 3 // sbnf c6 3 // sbnf2c9 3 // sbnf3c12 3 // snb c15 2 Notice how there is an overallocation of the constants: for sbnf there are 3 float AND one bool registers allocated. Now, checking again +d3dx output but tracing some more stuff: trace:d3dx:parse_ctab_constant_type name sbnf, elements 1, defaultvalue 0x18e1d0 trace:d3dx:parse_ctab_constant_type regset D3DXRS_FLOAT4, regidx 6, regcount 3 trace:d3dx:parse_ctab_constant_type class D3DXPC_STRUCT, type D3DXPT_VOID, rows 1, columns 3, elements 1, struct_members 3 trace:d3dx:parse_ctab_constant_type name b4, elements 1, defaultvalue 0x18e1d0 trace:d3dx:parse_ctab_constant_type regset D3DXRS_FLOAT4, regidx 6, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_BOOL, rows 1, columns 1, elements 1, struct_members 0 trace:d3dx:parse_ctab_constant_type name n4, elements 1, defaultvalue 0x18e1e0 trace:d3dx:parse_ctab_constant_type regset D3DXRS_FLOAT4, regidx 7, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_INT, rows 1, columns 1, elements 1, struct_members 0 trace:d3dx:parse_ctab_constant_type name f4, elements 1, defaultvalue 0x18e1f0 trace:d3dx:parse_ctab_constant_type regset D3DXRS_FLOAT4, regidx 8, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_FLOAT, rows 1, columns 1, elements 1, struct_members 0 trace:d3dx:parse_ctab_constant_type name sbnf, elements 1, defaultvalue 0x18e124 trace:d3dx:parse_ctab_constant_type regset D3DXRS_BOOL, regidx 8, regcount 1 trace:d3dx:parse_ctab_constant_type class D3DXPC_STRUCT, type D3DXPT_VOID, rows 1, columns 3, elements 1, struct_members 3 trace:d3dx:parse_ctab_constant_type name b4, elements 1, defaultvalue 0x18e124 trace:d3dx:parse_ctab_constant_type regset D3DXRS_BOOL, regidx 8, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_BOOL, rows 1, columns 1, elements 1, struct_members 0 trace:d3dx:parse_ctab_constant_type name n4, elements 1, defaultvalue 0x18e128 trace:d3dx:parse_ctab_constant_type regset D3DXRS_BOOL, regidx 9, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_INT, rows 1, columns 1, elements 1, struct_members 0 trace:d3dx:parse_ctab_constant_type name f4, elements 1, defaultvalue 0x18e12c trace:d3dx:parse_ctab_constant_type regset D3DXRS_BOOL, regidx 9, regcount 0 trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type D3DXPT_FLOAT, rows 1, columns 1, elements 1, struct_members 0 Sure enough, if you call GetConstantDesc() for sbnf, passing an array of two D3DXCONSTANT_DESC and setting the count to 2, you get both filled. Same with its struct members. Here's what I get from a quickly hacked test with native d3dx9: shader.c:6243: sbnf (0) registerset is 2. shader.c:6244: sbnf (0) registerindex is 6. shader.c:6245: sbnf (0) registercount is 3. shader.c:6243: sbnf (1) registerset is 0. shader.c:6244: sbnf (1) registerindex is 8. shader.c:6245: sbnf (1) registercount is 1. shader.c:6243: sbnf.b4 (0) registerset is 2. shader.c:6244: sbnf.b4 (0) registerindex is 6. shader.c:6245: sbnf.b4 (0) registercount is 1. shader.c:6243: sbnf.b4 (1) registerset is 0. shader.c:6244: sbnf.b4 (1) registerindex is 8. shader.c:6245: sbnf.b4 (1) registercount is 1. shader.c:6243: sbnf.n4 (0) registerset is 2. shader.c:6244: sbnf.n4 (0) registerindex is 7. shader.c:6245: sbnf.n4 (0) registercount is 1. shader.c:6243: sbnf.n4 (1) registerset is 0. shader.c:6244: sbnf.n4 (1) registerindex is 9. shader.c:6245: sbnf.n4 (1) registercount is 0. shader.c:6243: sbnf.f4 (0) registerset is 2. shader.c:6244: sbnf.f4 (0) registerindex is 8. shader.c:6245: sbnf.f4 (0) registercount is 1. shader.c:6243: sbnf.f4 (1) registerset is 0. shader.c:6244: sbnf.f4 (1) registerindex is 9. shader.c:6245: sbnf.f4 (1) registercount is 0. The 0 or 1 in parentheses indicate that the line refers to the first or the second D3DXCONSTANT_DESC
Re: [PATCH 4/5] d3dx9/tests: Add ID3DXConstantTable struct test.
2013/8/1 Rico Schüller kgbric...@web.de:
On 31.07.2013 00:14, Matteo Bruni wrote:
2013/7/30 Rico Schüller kgbric...@web.de:
Hi Matteo,
please see the attached patch.
On 25.07.2013 16:13, Matteo Bruni wrote:
2013/7/24 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/tests/shader.c | 308
+++
1 file changed, 308 insertions(+)
This is okay, but as a followup can you add some tests with mixed-type
structs? Something like:
struct
{
float f;
int i;
bool b;
};
If you have already written tests of this kind, I'd like to know what
does the compiler do in this case :)
Single variables could only have the tested types (I was not able to
generate other conversions than bool-bool, int-int, int-float,
bool-float, float-float). But I found a way to do it with structs and
there I found some issues. Hence this has to be fixed in wine, too.
Thanks
for the nice question. :-)
Basically you got these for the struct:
1. D3DXRS_FLOAT4: if one variable is used as float or a float variable is
used or an int variable is used as bool (the compiler may do some
optimization), else #2
2. D3DXRS_BOOL: if a bool variable is used as bool (in an if clause),
else
#3
3. D3DXRS_INT4
It looks like you could only do it that way with unused variables. I'm
not
sure if this makes sense at all. Why would someone set an unused
variable?
Maybe I missed something? Do you know anything else?
It does make some sense, although this is not what I expected. Also,
I'm getting different results...
If I understand correctly your test, all the fields of a structure
share the same registerset. Which is silly, since AFAIU each member of
the structure has a separate D3DXCONSTANT_DESC in the constant table,
both on disk and in memory, there is no point the compiler should
force the same registerset for all the struct members.
Yes, they share all the same registerset. The optimization seems to force
only the type of the register (it optimizes the not used members out).
Under the constraint of forcing all the members in the same
registerset, the conversion rules you mention make sense. In SM3 an
if bool can be replaced by an if_comp with a float register and a
rep/loop, which is controlled by an integer constant, can be emulated
via loop unrolling (although I'm not sure how the compiler can
possibly do that for the shader in your testcase). These are also
pretty much the only use cases of bool and int constants and there are
no int or bool non-constant registers so essentially no other type
conversion is possible. You can check the plain text output from fxc
to see how those constants are used in the shader code and how did the
compiler manage to convert those constants from one type to another.
I tried to compile your HLSL shader myself (I had to disable
optimization though, otherwise compilation fails) and, assuming the
text output of fxc matches what actually ends up in the shader
bytecode, in general I'm getting different struct members in different
registersets. E.g. snbf gets allocated to c6-c9 and b8. FWIW, I used
fxc from the June 2010 DirectX SDK, on Windows 7.
I'm not sure why my results are different from yours. Or am I
misunderstanding the test?
The bytecode should match the result in the text output. Also after some
additional test these structs seem to set several registers (as could be
seen in the text), so the constant table constant desc is not able to give a
full description for these. So an app should not depend on those or did I
miss something? It's getting tricky again.
E.g.:
struct {float f2; int n2; bool b2;} snb;
sets:
D3DXRS_FLOAT4 15,16 (60-68)
D3DXRS_BOOL 3,4,5 (3-6)
I'll send an update to the tests to cover these issue. I think I have to
look at the binary and whats really in there. Hopefully that information
could be found somewhere ...
The constant table could easily represent the situation by specifying
different register sets for different struct members.
But apparently the generated constant table is just broken. I ran the
test with WINEDEBUG=d3dx and this is an excerpt of the trace I got:
trace:d3dx:parse_ctab_constant_type name sbnf, elements 1, index 6,
defaultvalue 0x18e1d0, regset D3DXRS_FLOAT4
trace:d3dx:parse_ctab_constant_type class D3DXPC_STRUCT, type
D3DXPT_VOID, rows 1, columns 3, elements 1, struct_members 3
trace:d3dx:parse_ctab_constant_type name b4, elements 1, index 6,
defaultvalue 0x18e1d0, regset D3DXRS_FLOAT4
trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type
D3DXPT_BOOL, rows 1, columns 1, elements 1, struct_members 0
trace:d3dx:parse_ctab_constant_type name n4, elements 1, index 7,
defaultvalue 0x18e1e0, regset D3DXRS_FLOAT4
trace:d3dx:parse_ctab_constant_type class D3DXPC_SCALAR, type
D3DXPT_INT, rows 1, columns 1, elements 1, struct_members 0
trace:d3dx:parse_ctab_constant_type name f4, elements 1, index 8,
defaultvalue 0x18e1f0
Re: d3dx9 [patch 1/2]: Implement D3DXCreatePolygon
2013/8/1 Nozomi Kodama nozomi.kod...@yahoo.com:
+vertices = HeapAlloc(GetProcessHeap(), 0, 2 * (sides + 1) *
sizeof(D3DXVECTOR3));
+if (!vertices)
+{
+TRACE(Not memory enough for vertex buffer\n);
+polygon-lpVtbl-Release(polygon);
+return E_OUTOFMEMORY;
+}
+
+hr = polygon-lpVtbl-LockVertexBuffer(polygon, D3DLOCK_DISCARD,
(VOID **)vertices);
Why are you allocating memory and immediately dropping track of it?
Recheck how ID3DXMesh::LockVertexBuffer works.
Also I'm having a deja-vu here, this patch looks suspiciously similar
(up to having this same issue) to one I've already reviewed in the
past.
Re: [PATCH 4/5] d3dx9/tests: Add ID3DXConstantTable struct test.
2013/7/30 Rico Schüller kgbric...@web.de:
Hi Matteo,
please see the attached patch.
On 25.07.2013 16:13, Matteo Bruni wrote:
2013/7/24 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/tests/shader.c | 308
+++
1 file changed, 308 insertions(+)
This is okay, but as a followup can you add some tests with mixed-type
structs? Something like:
struct
{
float f;
int i;
bool b;
};
If you have already written tests of this kind, I'd like to know what
does the compiler do in this case :)
Single variables could only have the tested types (I was not able to
generate other conversions than bool-bool, int-int, int-float,
bool-float, float-float). But I found a way to do it with structs and
there I found some issues. Hence this has to be fixed in wine, too. Thanks
for the nice question. :-)
Basically you got these for the struct:
1. D3DXRS_FLOAT4: if one variable is used as float or a float variable is
used or an int variable is used as bool (the compiler may do some
optimization), else #2
2. D3DXRS_BOOL: if a bool variable is used as bool (in an if clause), else
#3
3. D3DXRS_INT4
It looks like you could only do it that way with unused variables. I'm not
sure if this makes sense at all. Why would someone set an unused variable?
Maybe I missed something? Do you know anything else?
It does make some sense, although this is not what I expected. Also,
I'm getting different results...
If I understand correctly your test, all the fields of a structure
share the same registerset. Which is silly, since AFAIU each member of
the structure has a separate D3DXCONSTANT_DESC in the constant table,
both on disk and in memory, there is no point the compiler should
force the same registerset for all the struct members.
Under the constraint of forcing all the members in the same
registerset, the conversion rules you mention make sense. In SM3 an
if bool can be replaced by an if_comp with a float register and a
rep/loop, which is controlled by an integer constant, can be emulated
via loop unrolling (although I'm not sure how the compiler can
possibly do that for the shader in your testcase). These are also
pretty much the only use cases of bool and int constants and there are
no int or bool non-constant registers so essentially no other type
conversion is possible. You can check the plain text output from fxc
to see how those constants are used in the shader code and how did the
compiler manage to convert those constants from one type to another.
I tried to compile your HLSL shader myself (I had to disable
optimization though, otherwise compilation fails) and, assuming the
text output of fxc matches what actually ends up in the shader
bytecode, in general I'm getting different struct members in different
registersets. E.g. snbf gets allocated to c6-c9 and b8. FWIW, I used
fxc from the June 2010 DirectX SDK, on Windows 7.
I'm not sure why my results are different from yours. Or am I
misunderstanding the test?
BTW, what needs to be fixed in Wine? I couldn't see anything obvious
by reading the test.
Cheers,
Matteo.
Cheers
Rico
Re: [1/1] kernel32: Implemented basic NUMA functions to replace the stubs
2013/7/29 Chris Moeller kod...@gmail.com: without looking very deeply into the CRT source code. You're talking about MS source code, right? That makes you tainted for contributing to Wine at the very least for C runtime-related code :( If you haven't looked into any other source code, this patch might be okay (barring other issues, I haven't read the patch at all). At least http://wiki.winehq.org/DeveloperFaq#head-fed5011434f62ae1a88baebfb8193a37ea795101 seems to suggest so.
Re: [1/1] kernel32: Implemented basic NUMA functions to replace the stubs
2013/7/30 Chris Moeller kod...@gmail.com: On Jul 29, 2013, at 11:22 AM, Matteo Bruni matteo.myst...@gmail.com wrote: 2013/7/29 Chris Moeller kod...@gmail.com: without looking very deeply into the CRT source code. You're talking about MS source code, right? That makes you tainted for contributing to Wine at the very least for C runtime-related code :( If you haven't looked into any other source code, this patch might be okay (barring other issues, I haven't read the patch at all). At least http://wiki.winehq.org/DeveloperFaq#head-fed5011434f62ae1a88baebfb8193a37ea795101 seems to suggest so. I did not look at any CRT source code to determine how to implement the functionality of this patch. I only looked at CRT source code to determine why the CRT was crashing in the first place. The HighestNode function popping a warning in my wine logs should have been hint enough to discover that function was stubbed. I actually determined how these functions worked by testing their behavior against a working system, and also with the aid of another developer, who pointed out a handy Win32 function for retrieving a suitable value for the free memory check. That's probably fine, although as a general rule you should stay as far as possible from Microsoft's source code. Cheers, Matteo.
Re: [PATCH 4/5] d3dx9/tests: Add ID3DXConstantTable struct test.
2013/7/24 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/tests/shader.c | 308
+++
1 file changed, 308 insertions(+)
This is okay, but as a followup can you add some tests with mixed-type
structs? Something like:
struct
{
float f;
int i;
bool b;
};
If you have already written tests of this kind, I'd like to know what
does the compiler do in this case :)
Re: [PATCH 3/5] d3dx9: Add support for structs to ID3DXConstantTable.
2013/7/24 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/shader.c | 79 -- 1 file changed, 77 insertions(+), 2 deletions(-) So there was actually a logic (an insane one, but yeah...). Have you found any game actually using this? Annoying question: have you checked if it's the same with newer d3dx9 dlls (e.g. d3dx9_43)? It still looks to me like buggy behavior and it might be that MS fixed that in later d3dx9 versions. This is not blocking the patch of course.
Re: [PATCH] wined3d: Implement per-stage constant in glsl fixed fonction pipeline.
2013/6/24 Christian Costa titan.co...@gmail.com:
Le 24/06/2013 09:24, Henri Verbeet a écrit :
On 23 June 2013 21:57, Christian Costa titan.co...@gmail.com wrote:
When D3DTA_CONSTANT is use in a texture stage, the generated shader uses
variables that are not defined making thus the compilation to fail.
This patch declare these variables with the value from the related
texture stage state D3D_TSS_CONSTANT.
This fixes the text display in Spin Tires demo.
This patch has several issues. Even without those, it should probably
be deferred anyway.
Even deferred, can you elaborate a bit so I can fix them. Unless you have
already something.
I'm not Henri but I can mention a number of issues (which might or
might not match with Henri's).
+for (stage = 0; stage MAX_TEXTURES settings-op[stage].cop
!= WINED3D_TOP_DISABLE; ++stage)
You probably want to generate this code only for texture stages
actually using constants.
+{
+float constant[4];
+
+constant[0] = ((settings-op[stage].constant 16) 0xff) / 255.0f;
+constant[1] = ((settings-op[stage].constant 8) 0xff) / 255.0f;
+constant[2] = ( settings-op[stage].constant 0xff) / 255.0f;
+constant[3] = ((settings-op[stage].constant 24) 0xff) / 255.0f;
No need to open code it, the macro D3DCOLORTOGLFLOAT4 does just that.
+
+shader_addline(buffer, const vec4 const%d = , stage);
+shader_glsl_append_imm_vec4(
buffer, constant);
+shader_addline(buffer, ;\n);
I assume it would be better to make these proper uniforms and update
their value in shader_glsl_load_constants() instead.
diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c
index 5b7fb3c..083a0d7 100644
--- a/dlls/wined3d/utils.c
+++ b/dlls/wined3d/utils.c
@@ -3312,6 +3312,8 @@ void gen_ffp_frag_op(const struct
wined3d_context *context, const struct wined3d
settings-op[i].aarg1 = aarg1;
settings-op[i].aarg2 = aarg2;
+settings-op[i].constant = state-texture_states[i][
WINED3D_TSS_CONSTANT];
+
if (state-texture_states[i][WINED3D_TSS_RESULT_ARG] == WINED3DTA_TEMP)
settings-op[i].dst = tempreg;
else
diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h
index dca0d61..c1d6c91 100644
--- a/dlls/wined3d/wined3d_private.h
+++ b/dlls/wined3d/wined3d_private.h
@@ -1673,6 +1673,8 @@ struct texture_stage_op
unsignedaarg2 : 8;
unsignedaarg0 : 8;
+DWORD constant;
+
struct color_fixup_desc color_fixup;
unsignedtex_type : 3;
unsigneddst : 1;
You don't need this if you use uniforms.
Also adding a test would be nice probably.
That said, maybe Henri already has a patch for it.
Re: po: Update Italian translation (try 3)
2013/6/17 Fabian Ebner f.ebne...@gmail.com: Typo fixed (finally) --- po/it.po | 40 ++-- 1 file changed, 14 insertions(+), 26 deletions(-) This one looks good to me, thanks :)
Re: Update Italian translation
2013/6/16 Fabian Ebner f.ebne...@gmail.com: --- po/it.po | 40 ++-- 1 file changed, 14 insertions(+), 26 deletions(-) #: joy.rc:42 msgid After disabling or enabling a device, the connected joysticks won't be updated here until you restart this applet. msgstr +Dopo disattivare o attivare un dispositivo, i joystick connessi non saranno +attualizzati qui, fino al riavvio dell'applet. That sounds like broken Italian (or a word-by-word translation), maybe something like Dopo aver disattivato o attivato un dispositivo, i joystick connessi non saranno aggiornati fino al riavvio dell'applet.? The original English message has probably some room for improvement too. #: jscript.rc:39 msgid 'return' statement outside of function -msgstr +msgstr Istruzione 'return' fouri dalla funzione Typo.
Re: po: Update Italian translation (try 2)
2013/6/17 Fabian Ebner f.ebne...@gmail.com: Fixed a typo and a sentence, thanks to Matteo --- po/it.po | 40 ++-- 1 file changed, 14 insertions(+), 26 deletions(-) #: jscript.rc:39 msgid 'return' statement outside of function -msgstr +msgstr Istruzione 'return' fouri della funzione Sorry, but this is worse...
Re: [PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.
2013/6/12 Christian Costa titan.co...@gmail.com:
+HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code,
D3DXSEMANTIC *semantics, UINT *count)
+{
+const DWORD *ptr = byte_code;
+UINT i = 0;
+
+TRACE(byte_code = %p, semantics = %p, count = %p\n, byte_code,
semantics, count);
+
+if (!byte_code)
+return D3DERR_INVALIDCALL;
+
+TRACE(Shader version: %#x\n, *ptr);
+
+/* Look for the END token, skipping the VERSION token */
+while (*++ptr != D3DSIO_END)
I think you should be a bit more careful in skipping non-opcode
DWORDs, otherwise you could e.g. potentially match with constants from
DEF instructions - very unlikely to happen in practice, sure, but
since it can be easily avoided, why not?
Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can
probably get away without having to write a table with the parameters
count for each SM1 opcode by just skipping DWORDs with the most
significant bit set (see shader_skip_unrecognized() from the same
file). Of course you still have to special case DEF but that should be
it.
I was thinking about this kind of problem but followed what
D3DXGetShaderSize does.
So D3DXGetShaderSize will have to be fixed as well.
So if I summarize:
if (sm 2) handle instruction length
else if (comment or def instruction) special handling for them
else skip DWORD with bit 31 set
Is this correct?
SM = 2 for the instruction length case, but apart from that it should be good.
Ideally we'd use some tests, as Henri suggested, but that's not
blocking this patch I think.
+{
+/* Skip comments */
+if ((*ptr D3DSI_OPCODE_MASK) == D3DSIO_COMMENT)
+{
+ptr += ((*ptr D3DSI_COMMENTSIZE_MASK)
D3DSI_COMMENTSIZE_SHIFT);
+}
+else if ((*ptr D3DSI_OPCODE_MASK) == D3DSIO_DCL)
+{
+DWORD param1 = *++ptr;
+DWORD param2 = *++ptr;
+DWORD usage = param1 0x1f;
+DWORD usage_index = (param1 16) 0xf;
+DWORD reg_type = (((param2 11) 0x3) 3) | ((param2
28) 0x7);
+
+TRACE(D3DSIO_DCL param1: %#x, param2: %#x, usage: %u,
usage_index: %u, reg_type: %u\n,
+ param1, param2, usage, usage_index, reg_type);
+
+if (reg_type == D3DSPR_INPUT)
+{
+if (semantics)
+{
+semantics[i].Usage = usage;
+semantics[i].UsageIndex = usage_index;
+}
+i++;
+}
+}
+}
+
+if (count)
+*count = i;
+
+return D3D_OK;
+}
Have you tried to implement D3DXGetShaderOutputSemantics too? I
suspect most of the code will be pretty much the same, in that case
you could move the common code to a helper function and use it from
both.
You don't necessarily need to do this right now, just mentioning a
potential follow-up.
I tought about it too. There are indeed similar. I planned to do it later
but I will do it in this patch since I have to update it anyway.
Thanks
Christian
Re: [PATCH] d3dx9_36: Implement D3DXGetShaderInputSemantics + tests.
2013/6/11 Christian Costa titan.co...@gmail.com:
Fixes bug 22682.
---
dlls/d3dx9_36/d3dx9_36.spec |2 +-
dlls/d3dx9_36/shader.c | 50 ++
dlls/d3dx9_36/tests/shader.c | 55
++
include/d3dx9shader.h|1 +
4 files changed, 107 insertions(+), 1 deletion(-)
diff --git a/dlls/d3dx9_36/d3dx9_36.spec b/dlls/d3dx9_36/d3dx9_36.spec
index f2c229a..0f1387e 100644
--- a/dlls/d3dx9_36/d3dx9_36.spec
+++ b/dlls/d3dx9_36/d3dx9_36.spec
@@ -159,7 +159,7 @@
@ stdcall D3DXGetPixelShaderProfile(ptr)
@ stdcall D3DXGetShaderConstantTable(ptr ptr)
@ stdcall D3DXGetShaderConstantTableEx(ptr long ptr)
-@ stub D3DXGetShaderInputSemantics(ptr ptr ptr)
+@ stdcall D3DXGetShaderInputSemantics(ptr ptr ptr)
@ stub D3DXGetShaderOutputSemantics(ptr ptr ptr)
@ stdcall D3DXGetShaderSamplers(ptr ptr ptr)
@ stdcall D3DXGetShaderSize(ptr)
diff --git a/dlls/d3dx9_36/shader.c b/dlls/d3dx9_36/shader.c
index 75bc9b5..3b71b4b 100644
--- a/dlls/d3dx9_36/shader.c
+++ b/dlls/d3dx9_36/shader.c
@@ -1,6 +1,7 @@
/*
* Copyright 2008 Luis Busquets
* Copyright 2009 Matteo Bruni
+ * Copyright 2010, 2013 Christian Costa
* Copyright 2011 Travis Athougies
*
* This library is free software; you can redistribute it and/or
@@ -1778,3 +1779,52 @@ HRESULT WINAPI D3DXGetShaderSamplers(const DWORD
*byte_code, const char **sample
return D3D_OK;
}
+
+HRESULT WINAPI D3DXGetShaderInputSemantics(const DWORD *byte_code,
D3DXSEMANTIC *semantics, UINT *count)
+{
+const DWORD *ptr = byte_code;
+UINT i = 0;
+
+TRACE(byte_code = %p, semantics = %p, count = %p\n, byte_code,
semantics, count);
+
+if (!byte_code)
+return D3DERR_INVALIDCALL;
+
+TRACE(Shader version: %#x\n, *ptr);
+
+/* Look for the END token, skipping the VERSION token */
+while (*++ptr != D3DSIO_END)
I think you should be a bit more careful in skipping non-opcode
DWORDs, otherwise you could e.g. potentially match with constants from
DEF instructions - very unlikely to happen in practice, sure, but
since it can be easily avoided, why not?
Take a look at shader_skip_opcode() from wined3d/shader_sm1.c. You can
probably get away without having to write a table with the parameters
count for each SM1 opcode by just skipping DWORDs with the most
significant bit set (see shader_skip_unrecognized() from the same
file). Of course you still have to special case DEF but that should be
it.
+{
+/* Skip comments */
+if ((*ptr D3DSI_OPCODE_MASK) == D3DSIO_COMMENT)
+{
+ptr += ((*ptr D3DSI_COMMENTSIZE_MASK)
D3DSI_COMMENTSIZE_SHIFT);
+}
+else if ((*ptr D3DSI_OPCODE_MASK) == D3DSIO_DCL)
+{
+DWORD param1 = *++ptr;
+DWORD param2 = *++ptr;
+DWORD usage = param1 0x1f;
+DWORD usage_index = (param1 16) 0xf;
+DWORD reg_type = (((param2 11) 0x3) 3) | ((param2 28)
0x7);
+
+TRACE(D3DSIO_DCL param1: %#x, param2: %#x, usage: %u,
usage_index: %u, reg_type: %u\n,
+ param1, param2, usage, usage_index, reg_type);
+
+if (reg_type == D3DSPR_INPUT)
+{
+if (semantics)
+{
+semantics[i].Usage = usage;
+semantics[i].UsageIndex = usage_index;
+}
+i++;
+}
+}
+}
+
+if (count)
+*count = i;
+
+return D3D_OK;
+}
Have you tried to implement D3DXGetShaderOutputSemantics too? I
suspect most of the code will be pretty much the same, in that case
you could move the common code to a helper function and use it from
both.
You don't necessarily need to do this right now, just mentioning a
potential follow-up.
diff --git a/dlls/d3dx9_36/tests/shader.c b/dlls/d3dx9_36/tests/shader.c
index f7be174..471cd8c 100644
--- a/dlls/d3dx9_36/tests/shader.c
+++ b/dlls/d3dx9_36/tests/shader.c
@@ -1,5 +1,6 @@
/*
* Copyright 2008 Luis Busquets
+ * Copyright 2010, 2013 Christian Costa
* Copyright 2011 Travis Athougies
*
* This library is free software; you can redistribute it and/or
@@ -271,6 +272,22 @@ static const D3DXCONSTANT_DESC ctab_samplers_expected[]
= {
{sampler2, D3DXRS_SAMPLER, 3, 1, D3DXPC_OBJECT, D3DXPT_SAMPLER3D, 1,
1, 1, 0, 4, NULL},
{notsampler, D3DXRS_FLOAT4, 2, 1, D3DXPC_VECTOR, D3DXPT_FLOAT, 1,
4, 1, 0, 16, NULL}};
+static const DWORD semantics_vs11[] = {
+0xfffe0101,
/* vs_1_1 */
+0x001f, 0x8000, 0x900f,
/* dcl_position0 v0 (input) */
+0x001f, 0x8005, 0x900f0001,
/* dcl_texcoord0 v1 (input) */
+0x001f, 0x80010005, 0x900f0002
Re: [PATCH] d3dcompiler: Add a LUT to find compilation targets info.
2013/5/2 Christian Costa titan.co...@gmail.com:
---
dlls/d3dcompiler_43/compiler.c| 101
+++--
dlls/d3dcompiler_43/d3dcompiler_private.h |1
2 files changed, 81 insertions(+), 21 deletions(-)
diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c
index 4942985..a6a9867 100644
--- a/dlls/d3dcompiler_43/compiler.c
+++ b/dlls/d3dcompiler_43/compiler.c
@@ -490,6 +490,66 @@ HRESULT WINAPI D3DAssemble(const void *data, SIZE_T
datasize, const char *filena
return hr;
}
+struct {
Please make it static const.
+const char *name;
+enum shader_type type;
+DWORD sm_major;
+DWORD sm_minor;
+DWORD level_major;
+DWORD level_minor;
+BOOL sw;
+BOOL support;
+} targets_info[] = {
+/* SM 1 */
+{ vs_1_0,ST_VERTEX, 1, 0, 0, 0, FALSE, TRUE },
+{ vs_1_1,ST_VERTEX, 1, 1, 0, 0, FALSE, TRUE },
+{ ps_1_0,ST_PIXEL, 1, 0, 0, 0, FALSE, TRUE },
+{ ps_1_1,ST_PIXEL, 1, 1, 0, 0, FALSE, FALSE },
+{ ps_1_2,ST_PIXEL, 1, 2, 0, 0, FALSE, FALSE },
+{ ps_1_3,ST_PIXEL, 1, 3, 0, 0, FALSE, FALSE },
+{ ps_1_4,ST_PIXEL, 1, 4, 0, 0, FALSE, FALSE },
+{ tx_1_0,ST_UNKNOWN, 1, 0, 0, 0, FALSE, FALSE },
+/* SM 2 */
+{ vs_2_0,ST_VERTEX, 2, 0, 0, 0, FALSE, TRUE },
+{ vs_2_a,ST_VERTEX, 2, 1, 0, 0, FALSE, FALSE },
+{ vs_2_sw, ST_VERTEX, 2, 0, 0, 0, TRUE, FALSE },
+{ ps_2_0,ST_PIXEL, 2, 0, 0, 0, FALSE, TRUE },
+{ ps_2_a,ST_PIXEL, 2, 1, 0, 0, FALSE, FALSE },
+{ ps_2_b,ST_PIXEL, 2, 2, 0, 0, FALSE, FALSE },
+{ ps_2_sw, ST_PIXEL, 2, 0, 0, 0, TRUE, FALSE },
+{ fx_2_0,ST_UNKNOWN, 2, 0, 0, 0, FALSE, FALSE },
+/* SM 3 */
+{ vs_3_0,ST_VERTEX, 3, 0, 0, 0, FALSE, TRUE },
+{ vs_3_sw, ST_VERTEX, 3, 0, 0, 0, TRUE, FALSE },
+{ ps_3_0,ST_PIXEL, 3, 0, 0, 0, FALSE, TRUE },
+{ ps_3_sw, ST_PIXEL, 3, 0, 0, 0, TRUE, FALSE },
+/* SM 4 */
+{ vs_4_0_level_9_0, ST_VERTEX, 4, 0, 9, 0, FALSE, FALSE },
+{ vs_4_0_level_9_1, ST_VERTEX, 4, 0, 9, 1, FALSE, FALSE },
+{ vs_4_0_level_9_3, ST_VERTEX, 4, 0, 9, 3, FALSE, FALSE },
+{ vs_4_0,ST_VERTEX, 4, 0, 0, 0, FALSE, TRUE },
+{ vs_4_1,ST_VERTEX, 4, 1, 0, 0, FALSE, TRUE },
+{ ps_4_0_level_9_0, ST_PIXEL, 4, 0, 9, 0, FALSE, FALSE },
+{ ps_4_0_level_9_1, ST_PIXEL, 4, 0, 9, 1, FALSE, FALSE },
+{ ps_4_0_level_9_3, ST_PIXEL, 4, 0, 9, 3, FALSE, FALSE },
+{ ps_4_0,ST_PIXEL, 4, 0, 0, 0, FALSE, TRUE },
+{ ps_4_1,ST_PIXEL, 4, 1, 0, 0, FALSE, TRUE },
+{ gs_4_0,ST_UNKNOWN, 4, 0, 0, 0, FALSE, FALSE },
+{ gs_4_1,ST_UNKNOWN, 4, 1, 0, 0, FALSE, FALSE },
+{ cs_4_0,ST_UNKNOWN, 4, 0, 0, 0, FALSE, FALSE },
+{ cs_4_1,ST_UNKNOWN, 4, 1, 0, 0, FALSE, FALSE },
+{ fx_4_0,ST_UNKNOWN, 4, 0, 0, 0, FALSE, FALSE },
+{ fx_4_1,ST_UNKNOWN, 4, 1, 0, 0, FALSE, FALSE },
+/* SM 5 */
+{ vs_5_0,ST_VERTEX, 5, 0, 0, 0, FALSE, TRUE },
+{ ps_5_0,ST_PIXEL, 5, 0, 0, 0, FALSE, TRUE },
+{ hs_5_0,ST_UNKNOWN, 5, 0, 0, 0, FALSE, FALSE },
+{ gs_5_0,ST_UNKNOWN, 5, 0, 0, 0, FALSE, FALSE },
+{ ds_5_0,ST_UNKNOWN, 5, 0, 0, 0, FALSE, FALSE },
+{ cs_5_0,ST_UNKNOWN, 5, 0, 0, 0, FALSE, FALSE },
+{ fx_5_0,ST_UNKNOWN, 5, 0, 0, 0, FALSE, FALSE },
+};
+
static HRESULT compile_shader(const char *preproc_shader, const char
*target, const char *entrypoint,
ID3DBlob **shader_blob, ID3DBlob **error_messages)
{
@@ -500,38 +560,37 @@ static HRESULT compile_shader(const char
*preproc_shader, const char *target, co
ID3DBlob *buffer;
char *pos;
enum shader_type shader_type;
+ULONG i;
+ULONG nb = sizeof(targets_info) / sizeof(targets_info[0]);
TRACE(Preprocessed shader source: %s\n, debugstr_a(preproc_shader));
TRACE(Parsing compilation target %s.\n, debugstr_a(target));
-if (strlen(target) != 6 || target[1] != 's' || target[2] != '_' ||
target[4] != '_')
+for (i = 0; i nb; i++)
{
-FIXME(Unknown compilation target %s.\n, debugstr_a(target));
-return D3DERR_INVALIDCALL;
+if (!strcmp(target, targets_info[i].name))
+{
+if (!targets_info[i].support)
+{
+FIXME(Compilation target %s not yet supported\n,
debugstr_a(target));
+return D3DERR_INVALIDCALL;
+}
+else
+{
+shader_type = targets_info[i].type;
+major =
Re: [PATCH 4/4] d3dx9_36: Remove todo in tests as it works now with last d3dxof fixes.
2013/4/29 Christian Costa titan.co...@gmail.com: --- dlls/d3dx9_36/tests/xfile.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/d3dx9_36/tests/xfile.c b/dlls/d3dx9_36/tests/xfile.c index 1238bcd..e514a5f 100644 --- a/dlls/d3dx9_36/tests/xfile.c +++ b/dlls/d3dx9_36/tests/xfile.c @@ -77,7 +77,7 @@ static void test_templates(void) ok(ret == D3DXFERR_BADFILEFLOATSIZE, RegisterTemplates returned %#x, expected %#x\n, ret, D3DXFERR_BADFILEFLOATSIZE); ret = d3dxfile-lpVtbl-RegisterTemplates(d3dxfile, templates_parse_error, sizeof(templates_parse_error) - 1); -todo_wine ok(ret == D3DXFERR_PARSEERROR, RegisterTemplates returned %#x, expected %#x\n, ret, D3DXFERR_PARSEERROR); +ok(ret == D3DXFERR_PARSEERROR, RegisterTemplates returned %#x, expected %#x\n, ret, D3DXFERR_PARSEERROR); ret = d3dxfile-lpVtbl-RegisterTemplates(d3dxfile, templates, sizeof(templates) - 1); ok(ret == S_OK, RegisterTemplates failed with %#x\n, ret); Hi Christian, good catch for that todo_wine. However, this should be merged with the patch actually fixing the test (I guess it's patch 2).
Re: [PATCH 1/2] wined3d: Set GL_NONE for glReadBuffer / glDrawBuffer on FBO initialization.
2013/4/25 Henri Verbeet hverb...@gmail.com: Seems ok, though a small comment explaining the issue this patch fixes probably wouldn't hurt. A comment in the commit message or in the code? On 25 April 2013 20:43, Matteo Bruni mbr...@codeweavers.com wrote: +if (target == GL_READ_FRAMEBUFFER) +context_bind_fbo(context, GL_DRAW_FRAMEBUFFER, draw_binding ? draw_binding : 0); +else +context_bind_fbo(context, GL_READ_FRAMEBUFFER, read_binding ? read_binding : 0); I don't think using 0 here is strictly wrong, but a proper NULL would probably be clearer. Indeed :/ As a followup, I think we can actually get rid of the glGenFramebuffers() in context_bind_fbo() these days and just pass a GLuint instead of a pointer. The only place left that actually creates FBOs should be context_apply_fbo_entry(). Good idea. I'll explore that. Thanks for the review, as usual.
Re: [PATCH 2/7] wined3d: Use ARB_internalformat_query2 to check for texture format rendering and blending support, where available.
Hrm, I sent the wrong version of the patch, I had actually moved the depth/stencil check earlier (so yeah, totally agree on that part). Also you're right about WINED3DFMT_FLAG_RENDERTARGET, for some reason I thought we were setting that in check_fbo_compat() but it's clearly not the case. I'm sending the updated patches in a bit (also dropping patches 5-7 since they are superseded by yours).
Re: d3dx9: Implement D3DXSHMultilpy5
2013/4/15 Nozomi Kodama nozomi.kod...@yahoo.com: Hello thanks for the review. I don't think that calling defines is the way to go. Indeed, I tested my patch and yours. Yours is about 12% slower than mine in my computer. And now, we try to take care of efficiency of this dll. So, it is not the time to increase latency. I used 10 digits since there are a lot of computation, I want to avoid as much as possible big rounding errors. If we want to uniformize, then we should uniformize d3dxshmultiply 2,3,4 with 10 digits. But that is for an another patch. Just to chip in about float precision. As already discussed, 32-bit float constants have about 7 decimals digits of precision, so extending these constants from 8 to 10 digits should not change anything. You would have to use doubles if you want more accurate values (but I don't think we should use doubles in d3dx9 dlls anyway). What you can do to ensure numeric accuracy is reviewing computations: the order of the operations (e.g. how you resolve operators associativity, etc) and storing/reusing intermediate results can make a difference. I wouldn't care about this stuff unless we get significantly different results compared to native. Nozomi. Looks pretty much ok, but isn't there a way to reduce the size a bit? Just see the dirty hack which is attached. D3DXSHMultiply6 will add a lot of lines too... Also is there a reason why we use constants with different accuracy (e.g. 0.28209479f in D3DXSHMultiply4 and 0.2820948064f)? Cheers Rico
Re: [PATCH 8/8] wined3d: Skip non-textureable formats in check_fbo_compat().
2013/4/10 Henri Verbeet hverb...@gmail.com: On 10 April 2013 01:35, Matteo Bruni mbr...@codeweavers.com wrote: --- dlls/wined3d/utils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dlls/wined3d/utils.c b/dlls/wined3d/utils.c index da8fc7b..5010676 100644 --- a/dlls/wined3d/utils.c +++ b/dlls/wined3d/utils.c @@ -1038,6 +1038,9 @@ static void check_fbo_compat(const struct wined3d_gl_info *gl_info, struct wined GLenum status; GLuint tex; +if (!(format-flags WINED3DFMT_FLAG_TEXTURE)) +return; + I'm not sure about this one, what exactly should this achieve? In either case, this probably belongs in init_format_fbo_compat_info(), together with the other similar checks. The idea is that, since check_fbo_compat() creates a texture of the specified format to attach it to a test FBO and verify that it works correctly, it doesn't make sense to even try if the format is non-texturable. The check can surely go to init_format_fbo_compat_info() instead.
Re: [PATCH 1/8] wined3d: Remove useless glClearxxx calls.
2013/4/10 Henri Verbeet hverb...@gmail.com: On 10 April 2013 01:35, Matteo Bruni mbr...@codeweavers.com wrote: -/* Clear the screen */ -gl_info-gl_ops.gl.p_glClearColor(1.0f, 0.0f, 0.0f, 0.0f); -checkGLcall(glClearColor); -gl_info-gl_ops.gl.p_glClearIndex(0); -gl_info-gl_ops.gl.p_glClearDepth(1); -gl_info-gl_ops.gl.p_glClearStencil(0x); - -checkGLcall(glClear); And actually potentially harmful, if the new context is created after an existing one already did drawing on the same drawable. (Although that's fairly unlikely, especially with AlwaysOffscreen enabled.) Notice that there isn't actually any glClear() call there. Interesting relics from the dawn of wined3d. :)
Re: [PATCH 1/2] wined3d: Handle texture types via ps/vs_compile_args
I guess it depends on what happens in D3D when the sampler type declared in the shader and the type of the texture bound to that sampler disagree, and it really matters only if all the current drivers have the same behavior in that regard. unbound_sampler_test() doesn't check what happens with mismatching texture and samplers and I don't think there is any test for that already (nor I remember if I wrote such a test previously). On current D3D drivers, sampling from NULL textures gives you black, where in the past there was no consistent behavior among driver vendors (with wined3d you would sample white most of the times). Applications started to depend on sampling black from NULL textures, so we introduced NULL-texture handling via black dummy textures. The current wined3d code works fine in that it behaves nicely from the OpenGL point of view and it doesn't cause bugs with D3D games, as far as I know, so I don't think there is any pressing reason to change that. Unless proved otherwise, of course... 2013/4/10 Stefan Dösinger stefandoesin...@gmail.com: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 2013-04-10 15:13, schrieb Henri Verbeet: On 10 April 2013 14:32, Stefan Dösinger ste...@codeweavers.com wrote: *) Shader declares a 2D sampler but e.g. a cube texture is bound Yes, but we have the dummy textures for that. My understanding was that we don't want to use the dummy textures for this purpose, but I'm not sure about the reason - probably because we have to re-bind the dummy texture every time the texture type is changed. Matteo, do you remember the details about this? If the dummy textures are the preferred way to handle incorrectly bound textures, most of this patch is moot. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRZWtoAAoJEN0/YqbEcdMwCvUP/1dUCAz4VGdGBMoJLCY19rlr yW9EXVuthizZXuoBueNPdBZtMj/NIh22WRr33aObWsMkvdL5vwuWklwMkHW/Pjzh xTQitfeu4wfI4DV9inCQaKiTWYxf9h+kAZxoaEnixGZHk5EkVUroic7n6hbGvPs+ e/kCwPyO+aYioxTFqvF2o4WANdzu0UF3MExz6GQiwqSzhNnlZhHy+hEDrOcbGbeS Eg9jgTutgFSEQu7+FDOHi8mNHQ7LCeBg09/OiUZVm00lTqS2FWFTg/7l1javKsdr FwKg6D7/CqdFmYSEv9aXzU+HaaDifNWI5P991qWb6k0i+IljpUnJHwy03vOcImHm VSQGTTHpFpWMAwDglbbBMU8cImhtd7EW9DmTH5kSIJ39ONCoYQRVZtXxGT+ducGa CbHDfgBbGBsP145+KtD35nAahf2+le9QwHMruBJ/Di53JcBUUuBYDIgNpJWz/wvK 7f1EcuMC05lMKxy6RE9dEE1CNrUpLz1IXVgHa3gDHl+cJRpBkWSV7uL0NDO7STS3 04PhDiQOwrnh7K48kMt7JztcKbG+obvdiOfv7Wj2kNPeRllMrQUN+AEaFKG3YQEP KeKsts6BBkjQkqYTYLCAm/vxCnJ9rWHmKdjjl355Il1aDocJb+mT48zZA5JBEzz9 flE2jibstgLp6Qzm7Wj1 =S7yF -END PGP SIGNATURE-
Re: [PATCH 8/8] wined3d: Skip non-textureable formats in check_fbo_compat().
2013/4/10 Henri Verbeet hverb...@gmail.com: On 10 April 2013 17:53, Matteo Bruni matteo.myst...@gmail.com wrote: The idea is that, since check_fbo_compat() creates a texture of the specified format to attach it to a test FBO and verify that it works correctly, it doesn't make sense to even try if the format is non-texturable. Well, we do check for format-glInternal in init_format_fbo_compat_info(), this would mostly affect formats like WINED3DFMT_P8_UINT or WINED3DFMT_UYVY that don't have WINED3DFMT_FLAG_TEXTURE set, but do have an internal format. I'm not sure we necessarily want to prevent doing e.g. FBO blits on those, or that we'd gain anything from not testing those. Right, if a format isn't supported at all by the GL implementation it should just get no internal format (and WINED3DFMT_FLAG_TEXTURE means something different). I guess I'll drop the patch then.
Re: d3dx9 [patch 1/2, resend]: Floatify fabs into fabs
2013/4/7 Nozomi Kodama nozomi.kod...@yahoo.com: Both patches are fine for me (note that the email subject is missing an f but the actual patch has the correct title).
Re: d3dx9 [patch1/3, try 2]: Do not use relative error for small numbers
2013/4/1 Nozomi Kodama nozomi.kod...@yahoo.com Since there are only at most 7 significant digits for a float, we must not be too picky about the expected difference for small numbers -#define relative_error(exp, out) ((exp == 0.0f) ? fabs(exp - out) : (fabs(1.0f - (out) / (exp +#define relative_error(exp, out) (fabs(exp) 0.01f ? fabs(exp - out) : fabs(1.0f - (out) / (exp))) This doesn't make much sense. Being floating point numbers, they have the same relative precision regardless of the magnitude of the value (as long as you are inside the range of representable values and excluding special values and denormals, but that should be irrelevant to our case here). If using a larger tolerance for the test in patch 2 makes it pass, then I think that's all you have to change. Otherwise you need to figure out why the test result differs so much from the expected value. Slightly unrelated, I think we should use fabsf() instead of fabs() there. We are already using fabsf() in a bunch of places, including d3dx9 tests, so there shouldn't be problems with that.
Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight
2013/3/15 Nozomi Kodama nozomi.kod...@yahoo.com: Patch series is fine to me.
Re: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight
2013/3/14 Nozomi Kodama nozomi.kod...@yahoo.com: No news about these patches? Nozomi? Hmm, I was under the impression that these patches were already picked up in git, but I see I was mistaken. Anyway they look okay to me, except for a bit in the tests from the first patch that I noticed just now by reading the patches again: +else if (j = order * order) +expected = test[l].red_received[j]; That somewhat defies the test, since you are then comparing a value with itself in the ok(). You can just use the original value (i.e. 1.01f + j for the red component) instead. Same issue with green and blue of course. Cheers, Matteo. De : Nozomi Kodama nozomi.kod...@yahoo.com À : wine-devel@winehq.org wine-devel@winehq.org Envoyé le : Lundi 11 mars 2013 17h11 Objet : RE: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight Hello Any problem with the patch http://source.winehq.org/patches/data/94777 and http://source.winehq.org/patches/data/94776 ? Nozomi.
Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalConeLight
2013/3/6 Nozomi Kodama nozomi.kod...@yahoo.com: hello Still problems with this patch? Nozomi. FWIW, both patches are fine to me.
Re: d3dx9 [patch 1/2]: Implement D3DXSHEvalConeLight
2013/3/1 Nozomi Kodama nozomi.kod...@yahoo.com: Why is this patch marked as not applied by the testbot? In my computer, with the latest git, it applies fine. Is there a problem with testbot? Nozomi Yeah, the old testbot has some issues. FWIW the patches applied just fine for me. Now, I know I'm going to be a PITA, but: +red_expected = test[l].red_out; ... +hr = D3DXSHEvalConeLight(order, dir, test[l].radius, 1.7f, 2.6f, 3.5f, red_expected, green_expected, blue_expected); ... +ok(relative_error(expected, red_expected[j]) admitted_error, isn't really better than the previous patch. What I meant is that the variables storing the output of D3DXSHEvalConeLight should be named in a way that shows it's the output of the function and what we want to test, while the variables with the expected values (the values a good implementation of the function should return and that you use to assess whether the implementation is working correctly) should be named accordingly too. So you went from calling everything out to calling everything expected, which doesn't help much ;) I'd just remove the red_expected variable and use test[l].red_out directly. Same for green and blue of course.
Re: d3dx9: Avoid expensive computations
2013/2/26 Rico Schüller kgbric...@web.de: Hi Nozomi, this is pretty fast. Just some numbers (run time on my machine, so it might not be that representative)... before: 43s previous patch: 27s this patch: 21s native: 16s So from the speed point of view, it's a lot closer than the rest. Though, I would split this into 2 patches, one for D3DXMatrixDeterminant and one for D3DXMatrixInverse. That's probably a good idea. I think it's a nice step forward. Thought we might test the speed of an sse version and may use it later ... Are there any other opinions? My main concern is that the effort in optimizing further those two functions might not have significant effects on actual application execution times (think diminishing returns). I'm not against making the code faster, especially if that doesn't make the code unreadable, but it might not be the best place to work on if you want to optimize d3dx9. You might want to profile some applications and see what the actual bottlenecks are. Specifically on these functions, an SSE-based version will probably run significantly faster, but you need to solve the issues with compatibility with older CPUs e.g. by selecting the correct function implementation at runtime in some fashion, as Henri mentioned. BTW there might be other potential problems, such as applications setting the SSE control register in some unexpected way (although that happens with the FPU control word too). You can also give a shot to GCC optimization options, such as -mfpmath=sse (and a suitable -march value). Obviously we don't want to use them in general but it might be interesting to see what GCC can do there. Keep in mind that the compiler has to stay on the safe side when optimizing and you might need to add attributes around to allow more aggressive optimizations. From a quick Google search I found http://locklessinc.com/articles/vectorize/ which seems to show the general idea. Cheers, Matteo. Cheers Rico On 25.02.2013 12:34, Nozomi Kodama wrote: Rico, can you give a try to this patch? If it is slightly slower than native, we could at first merge it. Anyway, if the application is well coded, this function should not be called often. Usually an application uses transformations matrices that are a lot easier to inverse Nozomi *De :* Henri Verbeet hverb...@gmail.com *À :* Rico Schüller kgbric...@web.de *Cc :* wine-devel@winehq.org; Nozomi Kodama nozomi.kod...@yahoo.com *Envoyé le :* Lundi 25 février 2013 0h08 *Objet :* Re: d3dx9: Avoid expensive computations On 25 February 2013 10:24, Rico Schüller kgbric...@web.de mailto:kgbric...@web.de wrote: I did some small tests for speed with the following results. You may also avoid such a lot of variable assignments like *pout = out and you may use 4 vecs instead. This should save ~48 assignments and it should also improve the speed a bit more (~10%). Though, native is still 40% faster than that. I'd somewhat expect native to use SSE versions of this kind of thing when the CPU supports those instructions. You also generally want to pay attention to the order in which you access memory, although perhaps it doesn't matter so much here because an entire matrix should be able to fit in a single cacheline, provided it's properly aligned.
Re: [PATCH 1/2] wined3d: Make (wined3d_)surface_depth_blt_fbo handle locations other than SFLAG_INTEXTURE.
2013/2/17 Henri Verbeet hverb...@gmail.com:
On 17 February 2013 18:10, Matteo Bruni mbr...@codeweavers.com wrote:
@@ -6168,18 +6172,10 @@ HRESULT surface_load_location(struct wined3d_surface
*surface, DWORD location, c
if (surface-resource.usage WINED3DUSAGE_DEPTHSTENCIL)
{
-if (location == SFLAG_INTEXTURE)
-{
-struct wined3d_context *context = context_acquire(device, NULL);
-surface_load_ds_location(surface, context, location);
-context_release(context);
-return WINED3D_OK;
-}
-else
-{
-FIXME(Unimplemented location %s for depth/stencil buffers.\n,
debug_surflocation(location));
-return WINED3DERR_INVALIDCALL;
-}
+struct wined3d_context *context = context_acquire(device, NULL);
+surface_load_ds_location(surface, context, location);
+context_release(context);
+return WINED3D_OK;
}
This is probably wrong (and the original code is probably questionable
too), surface_load_ds_location() is only really meant for onscreen -
offscreen blits. The original code is meant for the case where
AlwaysOffscreen is disabled and a depth stencil is used as texture.
The implication there is that the only possible locations are
SFLAG_INTEXTURE and SFLAG_INDRAWABLE, it doesn't really work with
multisampling. Fortunately you can't directly texture from
multisampled depth stencil surfaces either.
In principle we could make surface_load_ds_location() the generic
function for depth / stencil location transfers, but currently it
isn't. If you're hitting it with the RESZ code it probably only works
by accident.
I don't think I actually need any call to surface_load_ds_location() for RESZ.
What happens is that surface_depth_blt_fbo() wants to make sure the
depth buffer is current. It does that by calling
surface_load_location() with SFLAG_INRB_MULTISAMPLE location, if the
depth buffer is multisampled, but in that case the depth buffer is up
to date already. You're right that it doesn't make sense to call
surface_load_ds_location() (in its current form, anyway) at all here,
so what about this instead?
From eb55dfc4ae7d62802db45b10e4929b9e1b79226a Mon Sep 17 00:00:00 2001
From: Matteo Bruni mbr...@codeweavers.com
Date: Wed, 18 Jan 2012 14:38:04 +0100
Subject: [PATCH 1/2] wined3d: Make (wined3d_)surface_depth_blt_fbo handle
locations other than SFLAG_INTEXTURE.
---
dlls/d3d9/tests/visual.c | 11 +++
dlls/wined3d/surface.c | 42 ++
2 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/dlls/d3d9/tests/visual.c b/dlls/d3d9/tests/visual.c
index 4d429c4..81926ab 100644
--- a/dlls/d3d9/tests/visual.c
+++ b/dlls/d3d9/tests/visual.c
@@ -13411,14 +13411,9 @@ static void multisampled_depth_buffer_test(IDirect3D9
*d3d9)
for (i = 0; i sizeof(expected_colors) / sizeof(*expected_colors); ++i)
{
D3DCOLOR color = getPixelColorFromSurface(readback,
expected_colors[i].x, expected_colors[i].y);
-if (i % 4 2)
-todo_wine ok(color_match(color, expected_colors[i].color, 1),
-Expected color 0x%08x at (%u, %u), got 0x%08x.\n,
-expected_colors[i].color, expected_colors[i].x,
expected_colors[i].y, color);
-else
-ok(color_match(color, expected_colors[i].color, 1),
-Expected color 0x%08x at (%u, %u), got 0x%08x.\n,
-expected_colors[i].color, expected_colors[i].x,
expected_colors[i].y, color);
+ok(color_match(color, expected_colors[i].color, 1),
+Expected color 0x%08x at (%u, %u), got 0x%08x.\n,
+expected_colors[i].color, expected_colors[i].x,
expected_colors[i].y, color);
}
hr = IDirect3DDevice9_Present(device, NULL, NULL, NULL, NULL);
diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c
index 57527e4..51a7578 100644
--- a/dlls/wined3d/surface.c
+++ b/dlls/wined3d/surface.c
@@ -1040,17 +1040,20 @@ static BOOL surface_is_full_rect(const struct
wined3d_surface *surface, const RE
return TRUE;
}
-static void wined3d_surface_depth_blt_fbo(const struct wined3d_device *device,
struct wined3d_surface *src_surface,
-const RECT *src_rect, struct wined3d_surface *dst_surface, const RECT
*dst_rect)
+static void surface_depth_blt_fbo(const struct wined3d_device *device,
+struct wined3d_surface *src_surface, DWORD src_location, const RECT
*src_rect,
+struct wined3d_surface *dst_surface, DWORD dst_location, const RECT
*dst_rect)
{
const struct wined3d_gl_info *gl_info;
struct wined3d_context *context;
DWORD src_mask, dst_mask;
GLbitfield gl_mask;
-TRACE(device %p, src_surface %p, src_rect %s, dst_surface %p, dst_rect
%s.\n,
-device, src_surface, wine_dbgstr_rect(src_rect),
-dst_surface, wine_dbgstr_rect(dst_rect
Re: d3dx9: Implement D3DXSHEvalConeLight
Hi,
in addition to what I already told you in #winehackers (that is,
please rename the test struct field red_in to red_out and red_out to
red_expected, or something like that), you should take the chance to
do some more cleanup. That means e.g. use const instead of CONST
as Rico already suggested and fix the indentation of the last comment
you're adding.
There are a few other things I don't really like but it's about code
style and my personal preferences, so you are free to ignore them.
I'd write the test structure declaration as:
struct
{
float *red_out, *green_out, *blue_out;
const float *red_expected, *green_expected, *blue_expected;
float radius, red_offset, green_offset, blue_offset;
}
test[] =
{
{rout, gout, bout, table, table[72], table[144], 0.5f, 1.01f,
1.02f, 1.03f},
...
(I'm sure Gmail will wrap and make this piece of code unreadable, but well...)
In general I'd also avoid using caps in variable names (e.g.
r_intensity instead of Rintensity).
Re: wined3d: Rebind texture before checking for its content in check_fbo_compat().
2013/1/26 Stefan Dösinger stefandoesin...@gmail.com: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 2013-01-26 22:32, schrieb Matteo Bruni: + gl_info-fbo_ops.glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, tex, 0); Do you really have to re-attach it to the FBO? Or does glBindTexture(GL_TEXTURE_2D, tex) work as well? Yeah, that works too. I've sent a new patch with this change. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRBFTlAAoJEN0/YqbEcdMw7bcQAIfUM81xBr3zIpcEYek3gMso jXPq49+8Dp8Y3xiJGU/nFD8yLH9Y24SXpyageueCMWZR2jnEQcXBL0l76MmL5ZYK Y/L1ytRwrOvdXP07pJ8p3ZXxmA6hl82fuNxMvYhlSYP8PVCoobhaH7MwGp9WPpGF Ij20eOOR3GQnDjb6M2qHHfP9s+mNREu5NrvZ76NCf/lURXLJyDECuMz+6TbSl4CM qv0xCKdtqxm13gTZio3ftKc3QZZUSstiYjUYQmFh5NXQe5oIr9j2/iueU2K0zA/M CqswN4divbFFbh7QwCIDaubwfGidRfiwIG3lCCiNpfIvnowQouELIoFKjIipQ1AL ihIIS7iyekasAxkErVZ3E8O8IJCKYbTC+tAXBfK3niKmSRfdUB9cmfo5nVZw4Xjb 3/Zz84WnjMlIznyBGsKyKX+VLKlqzXY5zCyboHuEWPxTXdS0FAbwHzz06Kv0zIqc Pe4X6dJOUz6EPnx2gkbtFygGFbAa8haWhiIcY01H1bMSa7NnZPU3QgQXimrvXaKK SbQofZfhbNSVv0dL1kxqUYeLPb5hKXqMRHO7yi/GeyejTie9onzjxrjt4zn4rxe3 9IEZ0ymUYs499ahUnW/DAe0yFJvX+EXiPIiWwLBGVpx/Dzduk0WADOLBaN/YwrEz 1gxkuQjF2tbVSTkP41yr =xi8z -END PGP SIGNATURE-
Re: [PATCH 3/3] d3dx9: Use all 32 bits as mask.
2013/1/10 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/texture.c | 6 +++--- 1 Datei geändert, 3 Zeilen hinzugefügt(+), 3 Zeilen entfernt(- -DWORD i, v; +DWORD i, v, mask32 = format-bits[c] == 32 ? -1 : ((1 format-bits[c]) - 1); Usually we prefer to use ~0U instead of -1 with unsigned variables. Nice cleanups and fixes for larger pixel formats. Is this for Alan Wake?
Re: [PATCH 1/7] d3dx9: Handle invalid byte code in D3DXFindShaderComment().
2013/1/1 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/shader.c | 16 +++-
dlls/d3dx9_36/tests/shader.c | 21 +
2 Dateien geändert, 32 Zeilen hinzugefügt(+), 5 Zeilen entfernt(-)
+static inline BOOL is_valid_bytecode(DWORD token)
+{
+token = 0x;
+return token == 0x || token == 0xFFFE;
+}
This series looks good to me, but I'd prefer you use lowercase
hexadecimal constants in the future.
Re: d3dcompiler_43: Avoid signed-unsigned integer comparisons
2013/1/1 Andrew Talbot andrew.tal...@talbotville.com:
Changelog:
d3dcompiler_43: Avoid signed-unsigned integer comparisons.
diff --git a/dlls/d3dcompiler_43/bytecodewriter.c
b/dlls/d3dcompiler_43/bytecodewriter.c
index d10f6bc..17289d2 100644
--- a/dlls/d3dcompiler_43/bytecodewriter.c
+++ b/dlls/d3dcompiler_43/bytecodewriter.c
@@ -609,7 +609,7 @@ static void write_declarations(struct bc_writer *This,
}
static void write_const(struct constant **consts, int num, DWORD opcode,
DWORD reg_type, struct bytecode_buffer *buffer, BOOL len) {
-DWORD i;
+int i;
DWORD instr_def = opcode;
const DWORD reg = (131) |
((reg_type D3DSP_REGTYPE_SHIFT)
D3DSP_REGTYPE_MASK) |
I think it's better if you turn the num parameter into an unsigned int
(or DWORD) instead.
Re: [PATCH] d3dx9_36/tests: Fix broken line test
2012/11/27 Detlef Riekenberg wine@web.de: While inspecting a test failure, i found only 3 possible results for the line test: - d3dx9_36.dll not present - all tests skipped line.c:146: Tests skipped: Failed to create IDirect3DDevice9 object 0x8876086c - refcount test failed line.c:113: Test failed: Got 5 references to device 0023EA60, expected 2 (of course with different device pointer) See: http://test.winehq.org/data/tests/d3dx9_36:line.html I prefer to mark the refcount as implementation detail and remove the refcount test. My second way to fix the test failure is the attached patch. Since the test was accepted to wine, there must be a machine somewhere with a refcount of 2, so i marked that result as broken. I think the test was accepted simply because it passed on Wine while it wasn't actually executed on WineTestBot. So yeah, removing the refcount test or adding a todo_wine is probably the way to go, adding a broken(ref == 2) doesn't seem to make much sense. Thanks for bringing this up. d3d is not my area, so please comment. -- By by ... Detlef --- dlls/d3dx9_36/tests/line.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/dlls/d3dx9_36/tests/line.c b/dlls/d3dx9_36/tests/line.c index dc0c747..6ed508b 100644 --- a/dlls/d3dx9_36/tests/line.c +++ b/dlls/d3dx9_36/tests/line.c @@ -110,7 +110,9 @@ static void test_create_line(IDirect3DDevice9* device) expect_mat(world, result); ref = IDirect3DDevice9_Release(return_device); -ok(ref == 2, Got %x references to device %p, expected 2\n, ref, return_device); +todo_wine +ok(broken(ref == 2) || ref == 5, +Got %x references to device %p, expected 5\n, ref, return_device); ref = ID3DXLine_Release(line); ok(ref == 0, Got %x references to line %p, expected 0\n, ref, line); -- 1.7.5.4
Re: ntdll: Fixed some heap allocation stalls
2012/11/2 Steaphan Greene steap...@gmail.com:
Running a game in wine showed it performing terribly. I traced this to the
fact that it allocates and deallocates tiny memory chunks over and over (I
suspect it's in C++ and passing things by value everywhere). This led to
huge stalls because the heap bins weren't fine-grained enough (these
differed in size in steps of 8 bytes, but the bins differed by 16+, so it
spent a lot of time searching each bin for a bigger block). I added more
fine-grained sizes to the smaller end of this, and now it runs faster in
wine than it does natively. :)
This was run on Debian squeeze, amd64.
Note, this is my first submission to wine in nearly 15 years. So, of
course, everything has changed with how this works now. Hope I have this
all right.
---
dlls/ntdll/heap.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index a9044714..eb7406b 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -116,7 +116,9 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0 );
/* Max size of the blocks on the free lists */
static const SIZE_T HEAP_freeListSizes[] =
{
-0x10, 0x20, 0x30, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x1000, ~0UL
+0x10, 0x18, 0x20, 0x28, 0x30, 0x38, 0x40, 0x48, 0x50, 0x58, 0x60, 0x68,
+0x70, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0, 0xB8, 0xC0, 0xC8,
+0xD0, 0xD8, 0xE0, 0E88, 0xF0, 0xF8, 0x100, 0x200, 0x400, 0x1000, ~0UL
};
#define HEAP_NB_FREE_LISTS
(sizeof(HEAP_freeListSizes)/sizeof(HEAP_freeListSizes[0]))
That 0E88 looks quite wrong ;)
Apart from that, although I'm not an expert for this code, this patch
looks reasonable to me. Maybe we don't want so many free lists, but I
don't see big downsides for that (e.g. the linear search can be
replaced by a binary search, if need be). Maybe adding a smaller list
at the start (e.g. 0x8) makes sense too?
Re: ntdll: Fixed some heap allocation stalls
2012/11/3 Steaphan Greene steap...@gmail.com:
On 11/03/2012 09:04 AM, Matteo Bruni wrote:
2012/11/2 Steaphan Greenesteap...@gmail.com:
Running a game in wine showed it performing terribly. I traced this to
the
fact that it allocates and deallocates tiny memory chunks over and over
(I
suspect it's in C++ and passing things by value everywhere). This led to
huge stalls because the heap bins weren't fine-grained enough (these
differed in size in steps of 8 bytes, but the bins differed by 16+, so it
spent a lot of time searching each bin for a bigger block). I added more
fine-grained sizes to the smaller end of this, and now it runs faster in
wine than it does natively. :)
This was run on Debian squeeze, amd64.
Note, this is my first submission to wine in nearly 15 years. So, of
course, everything has changed with how this works now. Hope I have this
all right.
---
dlls/ntdll/heap.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index a9044714..eb7406b 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -116,7 +116,9 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0
);
/* Max size of the blocks on the free lists */
static const SIZE_T HEAP_freeListSizes[] =
{
-0x10, 0x20, 0x30, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x1000,
~0UL
+0x10, 0x18, 0x20, 0x28, 0x30, 0x38, 0x40, 0x48, 0x50, 0x58, 0x60,
0x68,
+0x70, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0, 0xB8, 0xC0,
0xC8,
+0xD0, 0xD8, 0xE0, 0E88, 0xF0, 0xF8, 0x100, 0x200, 0x400, 0x1000,
~0UL
};
#define HEAP_NB_FREE_LISTS
(sizeof(HEAP_freeListSizes)/sizeof(HEAP_freeListSizes[0]))
That 0E88 looks quite wrong ;)
Apart from that, although I'm not an expert for this code, this patch
looks reasonable to me. Maybe we don't want so many free lists, but I
don't see big downsides for that (e.g. the linear search can be
replaced by a binary search, if need be). Maybe adding a smaller list
at the start (e.g. 0x8) makes sense too?
Yep, that's a typo. Don't know how that got past me. Sorry. Should I
resend a corrected version?
Yes. You should also add a (try 2) to the email subject.
0x08 was left off because these values include the overhead of the arena
info, so the smallest requested size of 8 actually searches for larger than
8 (12, I think).
Ah, good point, I misread the code. Yes, it makes perfectly sense as it is.
I did try with fewer (every 16 instead of every 8), and, though it was still
a dramatic improvement, it was still slow.
I was thinking about e.g. going every 16 after 0x80, or some similar
pseudo-exponential growing, but that really depends on the allocation
pattern of the applications.
Speaking of which, it might be a nice followup patch to add some free
lists usage stats, to get some idea of what different applications do
in that regard.
Re: [3/5] d3dx9: Introduce a function for copying pixels.
2012/10/29 Christian Costa titan.co...@gmail.com:
pixel_format = get_format_info(src_desc.Format);
-if (pixel_format-type != FORMAT_ARGB)
-{
-FIXME(Unsupported pixel format %#x\n, src_desc.Format);
-return E_NOTIMPL;
-}
+if (pixel_format-type == FORMAT_UNKNOWN) return E_NOTIMPL;
Doesn't this still require a FIXME or at least a WARN?
There is still a FIXME in get_format_info(), that's enough I guess.
Re: [PATCH 3/5] wined3d: Implement WINED3DSIH_ROUND_NI in the GLSL shader backend.
2012/10/18 Henri Verbeet hverb...@codeweavers.com: --- dlls/wined3d/glsl_shader.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 57119e1..993b547 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -2530,6 +2530,7 @@ static void shader_glsl_map2gl(const struct wined3d_shader_instruction *ins) case WINED3DSIH_EXP: instruction = exp2; break; case WINED3DSIH_DSX: instruction = dFdx; break; case WINED3DSIH_DSY: instruction = ycorrection.y * dFdy; break; +case WINED3DSIH_ROUND_NI: instruction = floor; break; Hmm, does ROUND_NI really translate to floor() (as opposed to round()) or is that because round() is not supported in GLSL 1.20? In the latter case maybe a FIXME would help.
Re: [PATCH 3/5] wined3d: Implement WINED3DSIH_ROUND_NI in the GLSL shader backend.
2012/10/19 Henri Verbeet hverb...@gmail.com: On 19 October 2012 15:20, Matteo Bruni matteo.myst...@gmail.com wrote: 2012/10/18 Henri Verbeet hverb...@codeweavers.com: --- dlls/wined3d/glsl_shader.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/dlls/wined3d/glsl_shader.c b/dlls/wined3d/glsl_shader.c index 57119e1..993b547 100644 --- a/dlls/wined3d/glsl_shader.c +++ b/dlls/wined3d/glsl_shader.c @@ -2530,6 +2530,7 @@ static void shader_glsl_map2gl(const struct wined3d_shader_instruction *ins) case WINED3DSIH_EXP: instruction = exp2; break; case WINED3DSIH_DSX: instruction = dFdx; break; case WINED3DSIH_DSY: instruction = ycorrection.y * dFdy; break; +case WINED3DSIH_ROUND_NI: instruction = floor; break; Hmm, does ROUND_NI really translate to floor() (as opposed to round()) or is that because round() is not supported in GLSL 1.20? In the latter case maybe a FIXME would help. ROUND_NI rounds toward -INF, which is what floor() does. You're probably thinking of ROUND_NE which would translate to roundEven(). Indeed...
Re: [PATCH 2/6] d3dcompiler: Improve a Nvidia GPU recognition fallback.
Ehh, this patch should have been called something like wined3d: Improve a Nvidia GPU recognition fallback., but well...
Re: d3dx9_36 [try 3]: Implement D3DXSHRotate
2012/9/4 Rico Schüller kgbric...@web.de: On 04.09.2012 13:19, Nozomi Kodama wrote: +FLOAT expected, in[100], out[100], ... +for (i = 0; i 49; i++) +in[i] = i + 1.01f; I guess this belongs together. I'd like to make some suggestions, please don't take it personally. When you got a comment on a patch, read it carefully. Think about it, if it makes sens to do the change (just adding what I've said is not always the best, it just gives you a hint, that you may improve some things, the list is not always complete and different people may have different suggestions) and try to improve your patch. After you are done, read your patch. Well mistakes happen, especially when you try to get something done really fast. (1) Take you some time and read it again and see if you've done that as best as possible and if you are happy with the code you've done, then submit it. If you've found a bug don't submit, fix it and go to #1. The earlier a bug is found the cheaper is it to solve it. I hope this helps a bit and it hopefully saves you some time to get a patch in the feature accepted. Thought I'd like to thank you for your patience and hopefully I haven't annoyed you and we'll see some good quality patches in the feature from you. :-) Cheers Rico I agree with Rico. It's hard to find issues in your own code, which makes it all the more important to check it thoroughly. Still, at least try to be careful with the code style: +static void RotateX(FLOAT *out, UINT order, FLOAT a, FLOAT *in) I think it's better to only use lower case for the (private) function name. Something like rotatex or rotate_x or whatever. +if ( order 2 ) +return; Please indent it correctly. +return; +} Unneeded. +FLOAT* WINAPI D3DXSHRotate(FLOAT *out, UINT order, CONST D3DXMATRIX *matrix, CONST FLOAT *in) Fix the FLOAT*. +return; +} Again. Also, the patch is full of trailing whitespaces, please fix those too.
Re: wined3d: Use backup swapchain DC for devices created with desktop window.
2012/9/3 Henri Verbeet hverb...@gmail.com: On 3 September 2012 00:51, Adam Jakubek ajaku...@gmail.com wrote: Hi, This patch causes wined3d to use backup swapchain DC when IDirect3DDevice9 is created using the desktop window. Windows allows to create such device as long its type is D3DDEVTYPE_REF or D3DDEVTYPE_NULLREF. Unpatched Wine will fail in IDirect3D9_CreateDevice method, since it incorrectly uses the X root window. Relevant d3d test case is included. Patch closes bug #18490. How is actual rendering to the desktop window supposed to work? Interpreting MSDN it looks like D3DDEVTYPE_NULLREF devices don't actually render anything. The intended use case probably is to be able to create and load resources which can be shared (as in D3D9Ex-like resource sharing) with the real D3D device. D3DDEVTYPE_REF would trigger the reference rasterizer, which AFAIK isn't usually installed on user systems, so maybe this one ends up being the same as NULLREF in the common case. Of course this needs some tests/research.
Re: [PATCH 4/4] wined3d: Improve post-pixelshader blending test. (Try 2)
2012/8/20 Lucas Zawacki lfzawa...@gmail.com: Hello, this patch caused a regression with Rayman 2, all the text in the menus is rendered wrong now. Should I open a bug or is this email enough? A demo of the game can be downloaded here ftp://ftp.ubisoft.com/Rayman2/rayman2high.zip Screenshot: http://dl.dropbox.com/u/2701879/images/rayman2.png Regression test: a488e574497d674631b4036d4b179ce349ddb764 is the first bad commit commit a488e574497d674631b4036d4b179ce349ddb764 Author: Matteo Bruni mbr...@codeweavers.com Date: Wed Aug 15 00:38:26 2012 +0200 wined3d: Improve post-pixelshader blending test. :04 04 1a61c91a146de3e7a5206ccde1cc239fa0771e40 a9759ba603b3b52f2feccc716daff3d875a90b74 M dlls That's probably bug 31490, the patch from that bug (which I sent to wine-patches as http://www.winehq.org/pipermail/wine-patches/2012-August/117143.html) should fix it. Sorry for the inconvenience...
Re: d3dx9_36: Implementation of D3DXSHRotateZ
2012/7/18 Nozomi Kodama nozomi.kod...@yahoo.com: Minor nitpicks, but nevertheless: -@ stub D3DXSHRotateZ(ptr long long ptr) +@ stdcall D3DXSHRotateZ(ptr long long ptr) ... +FLOAT* WINAPI D3DXSHRotateZ(FLOAT *out, UINT order, FLOAT angle, CONST FLOAT *in) I guess using float in the spec file for the third parameter would be more appropriate (yes, it was already wrong but that's not an excuse ;) +if ( i = order * order || ( (order D3DXSH_MAXORDER) ( i = D3DXSH_MAXORDER * D3DXSH_MAXORDER ) ) ) +expected = ( i + 1.0f ) * ( i + 1.0f ); Bad indentation. In general the inner for loop in the tests feels awkward, try to rewrite it to avoid the duplicated stuff. +} + } +} You're adding a tab character here, please use only whitespaces for indentation. That said, thanks for your work on d3dx9!
Re: [PATCH 5/5] d3dcompiler: Implement basic expressions parsing.
2012/7/17 Stefan Dösinger stefandoesin...@gmx.at: Am Montag, 16. Juli 2012, 20:39:23 schrieb Matteo Bruni: +HLSL_IR_BINOP_MUL, ... +HLSL_IR_BINOP_DOT, HLSL has 3 somewhat related multiplication operations: *, mul and dot. Dot is fairly straightforward, but * and mul have subtle differences. I recommend to write tests to find out the exact details, but as far as I remember float4 * float4 gives you a float4 with a component-wise multiplication, whereas mul(float, float) returns a scalar like the dot product. float4x4 * float4 is an error, but mul(float4x4, float4) performs a matrix-vector multiplication and returns a float4. Indeed this is not very clear, but with HLSL_IR_BINOP_MUL I mean the translation of the * operator, while I'm planning to handle the mul() intrinsic with some code translating that into a bunch of MULs, ADDs and DOTs depending on the specific data types. mul() is apparently documented quite okay in MSDN but I know I'll have to test all of that anyway... +static struct hlsl_type *expr_common_type(struct hlsl_type *t1, struct hlsl_type *t2, +struct source_location *loc) This type inference system might be a bit too limited, you have to take the operation into account. e.g. a dot(float3, float4) returns a scalar and crs(float4, float4) returns a float3. Yeah, you're right, I'll need to take into account the operation in some cases, I consider this as a first approximation. However I was planning to handle the intrinsic function parameters / return value case you mention in a different way, i.e. by explicitly generating all the intrinsic functions prototypes, with all the data type combinations allowed, and letting the (currently missing) overloaded functions type checker/dispatcher deal with the data types. Not that I have written that code yet, so it may reveal to be a bad idea in the end. Stay tuned...
Re: [PATCH 3/4] d3dx9/tests: Add effect parameter value GetMatrixPointerArray() test.
2012/7/8 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/tests/effect.c | 90
++
1 files changed, 90 insertions(+), 0 deletions(-)
Hi Rico,
maybe it's just me misunderstanding this, but:
+FLOAT fvalue[EFFECT_PARAMETER_VALUE_ARRAY_SIZE];
+D3DXMATRIX *matrix_pointer_array[sizeof(fvalue)/sizeof(D3DXMATRIX)];
+UINT l, k, m, element;
+
+for (element = 0; element = res_desc-Elements + 1; ++element)
+{
+memset(fvalue, 0xab, sizeof(fvalue));
+for (l = 0; l element; ++l)
+{
+matrix_pointer_array[l] = (D3DXMATRIX *)fvalue[l *
sizeof(**matrix_pointer_array) / sizeof(*matrix_pointer_array)];
+}
I don't quite understand what you're trying to achieve with
sizeof(**matrix_pointer_array) / sizeof(*matrix_pointer_array). As
far as I can see, that depends on the size of the pointer (32/64 bits)
and I'm not sure that the offset should change in that case. Maybe you
meant to use sizeof(float) as divisor?
Re: [1/5] d3dx9: Implement D3DXLoadVolumeFromMemory.
2012/6/27 Józef Kucia joseph.ku...@gmail.com:
---
Hi Józef,
+void copy_simple_data(const BYTE *src, UINT srcpitch, SIZE src_size, const
PixelFormatDesc *srcformat,
+BYTE *dest, UINT destpitch, SIZE dst_size, const PixelFormatDesc
*destformat, D3DCOLOR colorkey) DECLSPEC_HIDDEN;
+
You are not actually using this function in the patch.
+ if (dst_box-Left dst_box-Right || dst_box-Right desc.Width)
+ return D3DERR_INVALIDCALL;
+ if (dst_box-Top dst_box-Bottom || dst_box-Bottom desc.Height)
+ return D3DERR_INVALIDCALL;
+ if (dst_box-Front dst_box-Back || dst_box-Back desc.Depth)
+ return D3DERR_INVALIDCALL;
+
+ dst_width = dst_box-Right - dst_box-Left;
+ dst_height = dst_box-Bottom - dst_box-Top;
+ dst_depth = dst_box-Back - dst_box-Front;
+ if (!dst_width || !dst_height || !dst_depth)
+ return D3DERR_INVALIDCALL;
You can avoid the last check by comparing e.g. (dst_box-Left =
dst_box-Right) above, instead of using .
+ if (src_box-Left (src_format_desc-block_width - 1)
+ || src_box-Top (src_format_desc-block_height - 1)
+ || (src_box-Right (src_format_desc-block_width - 1)
+ src_width != desc.Width)
+ || (src_box-Bottom (src_format_desc-block_height - 1)
+ src_height != desc.Height))
+ {
+ WARN(Source box (%u, %u, %u, %u) is misaligned\n,
+ src_box-Left, src_box-Top, src_box-Right,
src_box-Bottom);
+ return D3DXERR_INVALIDDATA;
+ }
I'm not quite sure of the right and bottom misalignment checks. If the
tests confirm that, of course that's okay.
+ for (slice = 0; slice src_depth; slice++)
+ {
+ src_addr = src_memory;
+ src_addr += slice * src_slice_pitch;
Shouldn't you take into account src_box-Front?
Re: [4/5] d3dx9: Implement D3DXCreateVolumeTextureFromFileInMemoryEx.
2012/6/27 Józef Kucia joseph.ku...@gmail.com:
---
+ for (mip_level = 0; mip_level mip_levels; mip_level++)
+ {
+ hr = calculate_dds_surface_size(src_info, width, height, src_pitch,
mip_level_size);
+ if (FAILED(hr)) return hr;
...
+ hr = D3DXLoadVolumeFromMemory(volume, palette, NULL, pixels,
src_info-Format,
+ src_pitch, mip_level_size, NULL, box, filter, color_key);
mip_level_size would be the source slice pitch, right? I'd just call
it like that.
Re: d3dx9_36 [patch 2 of 3]: Implementation of D3DXSHScale
2012/6/27 Nozomi Kodama nozomi.kod...@yahoo.com:
-@ stub D3DXSHScale(ptr long ptr ptr)
+@ stdcall D3DXSHScale(ptr long ptr ptr)
...
+FLOAT* WINAPI D3DXSHScale(FLOAT *out, UINT order, CONST FLOAT *a,
CONST FLOAT scale)
The last parameter doesn't match between the function prototype and
the spec entry. Also, apparently MSDN agrees with your prototype,
while the docs in the last DirectX SDK have a pointer there. I guess
the tests demonstrate that the parameter is actually a plain float...
+for (i = 0; i 100; i++)
+{
+a[i] = (FLOAT)i;
+b[i] = (FLOAT)i;
+}
No need to cast.
+D3DXSHScale(b, order, a, 5.0f);
Not important, but you could also verify that the returned pointer is == b.
+/* D3DXSHScale does not modify the elements of the array after the
order * order-th element */
Can you please indent those comments?
+return;
+}
That's not needed at all.
Also, while you're at it, you should probably merge the two for loops
in the test, like Francois did in
http://www.winehq.org/pipermail/wine-patches/2012-June/115464.html
Re: d3dx9_36 [patch 1of 2]: Implementation of D3DXSHDot
2012/6/19 Nozomi Kodama nozomi.kod...@yahoo.com:
+static void test_D3DXSHDot(void)
+{
+unsigned int i;
+FLOAT a[90], b[90], got;
+CONST FLOAT expected[] =
+{ 0.0f, 0.0f, 17.0f, 222.0f, 1300.0f, 5050.0f, 15225.0f,
38612.0f, 86352.0f, 175500.0f, };
+
+for (i = 0; i 90; i++)
+{
+a[i] = (FLOAT)i;
+b[i] = (FLOAT)i + 0.5f;
+}
+
+/* D3DXSHDot computes by using order * order elements */
+for (i = 0; i 9; i++)
+{
+got = D3DXSHDot(i, a, b);
+ok(relative_error(got, expected[i]) admitted_error, order
%d: expected %f, received %f\n, i, expected[i], got);
+}
+
+return;
+}
+
Making the a and b arrays 90 elements big doesn't make much sense.
If your D3DXSHDot implementation is correct, for the i = 8 case you
only need 8 * 8 = 64 elements. Also notice that the expected array
has 10 elements, while you use only 9.
Also please recheck formatting and whitespaces of this patch and patch 2/2.
Re: Re : d3dx9_36: D3DXQuaternionLn computes as if the norm of the input is 1
2012/6/13 Nozomi Kodama nozomi.kod...@yahoo.com: 2012/6/12 Nozomi Kodama nozomi.kod...@yahoo.com: + if ( (pq-w = 1.0f) || (pq-w == -1.0f) ) I think the second comparison should be '=', if you want to avoid getting NaNs. I checked in Vista. D3DX accepts -1.0f as input and returns what the patch does. However, any value -1.0f is a unacceptable value for D3DX ( D3DX returns (-1#IND00,-1#IND00,-1#IND00,-1#IND00) ). Best regards Nozomi Oh, so it actually returns NaNs. Interesting... So the patch is fine as it is. Maybe you could try to add such a case to the tests, if it doesn't get too tricky. You could check for something like gotquat.x != gotquat.x, which should return true only if gotquat.x is NaN.
Re: d3dx9_36: D3DXQuaternionLn computes as if the norm of the input is 1
2012/6/12 Nozomi Kodama nozomi.kod...@yahoo.com: +if ( (pq-w = 1.0f) || (pq-w == -1.0f) ) I think the second comparison should be '=', if you want to avoid getting NaNs.
Re: po: Update Italian translation
Hi Luca, just a couple of nitpicks: -Nota: è raccomandato usare i pacchetti delle distribuzioni. Leggi a href= -\http://wiki.winehq.org/Gecko\;http://wiki.winehq.org/Gecko/a per i -dettagli. +Nota: è raccomandato usare i pacchetti delle distribuzioni. Leggi a +href=\http://wiki.winehq.org/Mono\;http://wiki.winehq.org/Mono/a +per i dettagli. This sounds a bit awkward, I'd go for something like Nota: si raccomanda di usare i pacchetti forniti dalla tua distribuzione. #: cryptui.rc:413 -#, fuzzy -#| msgid Personal Information Exchange/PKCS #12 (.pfx) msgid Personal Information Exchange/PKCS #12 (*.pfx) -msgstr Scambia di Informazioni Personali/PKCS #12 (.pfx) +msgstr Scambia di Informazioni Personali/PKCS #12 (*.pfx) Scambio. #: joy.rc:33 msgid Joysticks -msgstr +msgstr Joysticks I'm quite sure foreign words are always wrote in singular form in Italian language, so this would be Joystick.
Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.
2012/6/5 Stefan Dösinger stefandoesin...@gmx.at:
Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni:
+struct hlsl_type
+{
...
+ unsigned int dimx;
+ unsigned int dimy;
...
+};
One thing I noticed when I wrote my compiler was that a float1x1 is not the
same as a float1 or float. I did not implement this difference nor do I know
if any games depend on it.
(That's no reason to hold up this patch, just something to keep in mind for
further work)
Yeah, I noticed that too, but it doesn't seem to make much of a
difference in practice. Of course, if later on it shows that it
actually matters for something relevant, I'll change things
accordingly.
Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.
2012/6/5 Rico Schüller kgbric...@web.de:
Am 05.06.2012 13:08, schrieb Matteo Bruni:
2012/6/5 Stefan Dösingerstefandoesin...@gmx.at:
Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni:
+struct hlsl_type
+{
...
+ unsigned int dimx;
+ unsigned int dimy;
...
+};
One thing I noticed when I wrote my compiler was that a float1x1 is not
the
same as a float1 or float. I did not implement this difference nor do I
know
if any games depend on it.
(That's no reason to hold up this patch, just something to keep in mind
for
further work)
Yeah, I noticed that too, but it doesn't seem to make much of a
difference in practice. Of course, if later on it shows that it
actually matters for something relevant, I'll change things
accordingly.
For the effect interface, it makes a difference. See
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l320
and
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l501
. The difference is the type D3DXPC_SCALAR vs D3DXPC_VECTOR vs
D3DXPC_MATRIX_ROWS and of course some different handling, see
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l1191
. A test may help to check how variables in shaders are handled.
Cheers
Rico
Re: [PATCH 5/5] d3dcompiler: Parse variable declarations.
2012/6/5 Matteo Bruni matteo.myst...@gmail.com:
2012/6/5 Rico Schüller kgbric...@web.de:
Am 05.06.2012 13:08, schrieb Matteo Bruni:
2012/6/5 Stefan Dösingerstefandoesin...@gmx.at:
Am Montag, 4. Juni 2012, 17:58:24 schrieb Matteo Bruni:
+struct hlsl_type
+{
...
+ unsigned int dimx;
+ unsigned int dimy;
...
+};
One thing I noticed when I wrote my compiler was that a float1x1 is not
the
same as a float1 or float. I did not implement this difference nor do I
know
if any games depend on it.
(That's no reason to hold up this patch, just something to keep in mind
for
further work)
Yeah, I noticed that too, but it doesn't seem to make much of a
difference in practice. Of course, if later on it shows that it
actually matters for something relevant, I'll change things
accordingly.
For the effect interface, it makes a difference. See
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l320
and
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l501
. The difference is the type D3DXPC_SCALAR vs D3DXPC_VECTOR vs
D3DXPC_MATRIX_ROWS and of course some different handling, see
http://source.winehq.org/git/wine.git/blob/90f93e3819dcaa0cea16bd297fa69a359fb6b517:/dlls/d3dx9_36/tests/effect.c#l1191
. A test may help to check how variables in shaders are handled.
Cheers
Rico
Oops, I misclicked on the send button, sorry for the spam...
I just did a quick test. It looks like that the actual generated code
does not change but the constant table is affected by the
scalar/vector/matrix type similarly to the effect interface stuff.
Since the compiler seems to preserve the type of the constant, I guess
in general this may matter, after all...
So I presume I'll add another field to the hlsl_type structure in the
near future to keep track of the scalar/vector/matrix type or
something like that. Anyway this patch is not affected by this detail,
so I think it is still good to go.
BTW, thanks for the comments :)
Re: ddraw: Forward AddAttachedSurface to the correct equivalent
2012/5/14 Stefan Dösinger stefandoesin...@gmx.at: Am Sonntag, 13. Mai 2012, 20:39:27 schrieb David Adam: + hr = ddraw_surface3_AddAttachedSurface(This- IDirectDrawSurface3_iface, + attachment_impl ? attachment_impl-IDirectDrawSurface3_iface : NULL); It isn't immediately clear to me why Surface3 is correct instead of Surface7. Do you have tests or bug reports to back this up? I think it is for http://bugs.winehq.org/show_bug.cgi?id=3467, but probably a test would still help. Also maybe it should be implemented the other way around i.e. surface3_AddAttachedSurface calling into surface4_AddAttachedSurface.
Re: [2/8] d3dx9: Check the size of a DDS file in D3DXGetImageInfoFromFileInMemory.
2012/5/9 Józef Kucia joseph.ku...@gmail.com:
+static HRESULT get_image_info_from_dds(const void *buffer, UINT length,
D3DXIMAGE_INFO *info)
{
+ UINT i;
+ UINT faces = 0;
...
+ /* calculate the expected length */
+ width = info-Width;
+ height = info-Height;
+ for (i = 0; i info-MipLevels; i++)
+ {
+ UINT pitch, size = 0;
+ calculate_dds_surface_size(info, width, height, pitch, size);
+ expected_length += size;
+ width = max(1, width / 2);
+ height = max(1, width / 2);
+ }
+
+ if (length expected_length)
+ return D3DXERR_INVALIDDATA;
It looks like you're not actually using the faces variable.
Re: [4/8] d3dx9: Implement D3DXCreateCubeTextureFromFileInMemoryEx.
2012/5/9 Józef Kucia joseph.ku...@gmail.com:
+ mip_levels = min(src_info-MipLevels,
IDirect3DCubeTexture9_GetLevelCount(cube_texture));
+ for (face = D3DCUBEMAP_FACE_POSITIVE_X; face =
D3DCUBEMAP_FACE_NEGATIVE_Z; face++)
+ {
+ size = src_info-Width;
+ for (mip_level = 0; mip_level mip_levels; mip_level++)
+ {
+ hr = calculate_dds_surface_size(src_info, size, size,
src_pitch, mip_level_size);
+ if (FAILED(hr)) return hr;
+
+ SetRect(src_rect, 0, 0, size, size);
+
+ IDirect3DCubeTexture9_GetCubeMapSurface(cube_texture, face,
mip_level, surface);
+ hr = D3DXLoadSurfaceFromMemory(surface, palette, NULL, pixels,
src_info-Format, src_pitch,
+ NULL, src_rect, filter, color_key);
+ IDirect3DSurface9_Release(surface);
+ if (FAILED(hr)) return hr;
+
+ pixels += mip_level_size;
+ size = max(1, size / 2);
+ }
+
+ /* if texture has fewer mip levels than DDS file, skip excessive mip
levels */
+ while (mip_level src_info-MipLevels)
+ {
+ calculate_dds_surface_size(src_info, size, size, src_pitch,
mip_level_size);
+ pixels += mip_level_size;
+ size = max(1, size / 2);
+ mip_level++;
+ }
+ }
Really a nitpick, you could merge the while into the for loop above.
+ if ((caps.Caps2 D3DCAPS2_DYNAMICTEXTURES) (pool != D3DPOOL_DEFAULT)
(usage != D3DUSAGE_DYNAMIC))
+ {
+ hr = D3DXCreateCubeTexture(device, size, mip_levels, usage, format,
pool, tex);
+ buftex = NULL;
+ }
+ else
+ {
+ hr = D3DXCreateCubeTexture(device, size, mip_levels, usage, format,
D3DPOOL_SYSTEMMEM, buftex);
+ tex = buftex;
+ }
The if condition doesn't look correct to me, since e.g. you don't
need a temporary texture if pool == D3DPOOL_DEFAULT and usage ==
D3DUSAGE_DYNAMIC.
Re: [5/8] d3dx9: Add DDS support in D3DXCreateTextureFromFile functions.
2012/5/9 Józef Kucia joseph.ku...@gmail.com: --- +HRESULT load_texture_from_dds(IDirect3DTexture9 *texture, const void *src_data, UINT srd_data_size, + const PALETTEENTRY *palette, DWORD filter, D3DCOLOR color_key, const D3DXIMAGE_INFO *src_info) DECLSPEC_HIDDEN; ... +HRESULT load_texture_from_dds(IDirect3DTexture9 *texture, const void *src_data, UINT srd_data_size, + const PALETTEENTRY *palette, DWORD filter, D3DCOLOR color_key, const D3DXIMAGE_INFO *src_info) I guess that was meant to be called src_data_size. More to the point, anyway, you're not using that value right now, you should probably add a check against it or remove the parameter altogether.
Re: [5/8] d3dx9: Add DDS support in D3DXCreateTextureFromFile functions.
2012/5/10 Józef Kucia joseph.ku...@gmail.com: I noticed that I put more and more texture functions tests in tests/surface.c, because these tests needs the dds files which are in tests/surface.c. I wonder if it would be better to copy needed dds files to tests/texture.c and move texture function tests to their proper place. I think the dds files could be used to tests other texture functions, e.g. D3DXCreateTextureFromFileInMemoryEx doesn't seem to have any tests. I think that's a good idea, yes.
Re: [PATCH 5/5] d3dcompiler: Partially implement D3DCompile function.
2012/5/8 Henri Verbeet hverb...@gmail.com: I realize compile_shader() is mostly a copy of assemble_shader(), but nevertheless: On 8 May 2012 16:17, Matteo Bruni mbr...@codeweavers.com wrote: +struct bwriter_shader *parse_hlsl_shader(const char *text, enum shader_type type, DWORD version, If this isn't called outside of compiler.c it should be static. This is going to change in the following patches, but yeah, right now it can be static. + LPD3DBLOB buffer; ID3DBlob * + CopyMemory(pos, preproc_messages, strlen(preproc_messages) + 1); memcpy() should do fine. + hr = SlWriteBytecode(shader, 9, res); ... + size = HeapSize(GetProcessHeap(), 0, res); + hr = D3DCreateBlob(size, buffer); That's not very pretty, SlWriteBytecode() should probably just return the size as well. True. I'll change that in assemble_shader() too, while I'm at it.
Re: [3/3] d3dx9: Implement conversion from DDS pixel format to D3DFORMAT.
Il 26 aprile 2012 11:47, Józef Kucia joseph.ku...@gmail.com ha scritto:
static D3DFORMAT dds_pixel_format_to_d3dformat(const struct dds_pixel_format
*pixel_format)
{
- FIXME(Pixel format conversion not implemented.\n);
+ if (pixel_format-flags DDS_PF_FOURCC)
+ return dds_fourcc_to_d3dformat(pixel_format-fourcc);
+ else if (pixel_format-flags DDS_PF_RGB)
+ return dds_rgb_to_d3dformat(pixel_format);
+ else if (pixel_format-flags DDS_PF_LUMINANCE)
+ return dds_luminance_to_d3dformat(pixel_format);
+ else if (pixel_format-flags DDS_PF_ALPHA_ONLY)
+ return dds_alpha_to_d3dformat(pixel_format);
You don't need all those 'else's, the 'return's imply that you get
there only when the 'if' condition isn't true.
The same applies in other areas of this patch.
I don't mind much, but since you have to resend the series anyway...
Re: d3dx9/tests: Add DDS pixel format tests for D3DXGetImageInfoFromFileInMemory.
Il 19 aprile 2012 19:37, Józef Kucia joseph.ku...@gmail.com ha scritto: --- dlls/d3dx9_36/tests/surface.c | 65 + 1 files changed, 65 insertions(+), 0 deletions(-) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 87aa03e..51d6ebd 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -22,6 +22,8 @@ #include wine/test.h #include d3dx9tex.h #include resources.h +#undef MAKE_DDHRESULT +#include ddraw.h I'm not sure that's a good idea. ddraw.h is not included in recent DirectX SDKs and the relevant definitions are gone, as far as I know. So I think d3dx9 or its tests should not depend on that header. I'd just (re)declare the stuff you need privately in the test source file instead. You don't even need to use the same types/defines/variables names as those from ddraw.h, you just have to keep them compatible. On that point, I see that MSDN mentions the equivalent DDS_HEADER and DDS_PIXELFORMAT structures (http://msdn.microsoft.com/en-us/library/windows/desktop/bb943991%28v=vs.85%29.aspx), except those don't seem to be defined in the MS headers either. I guess you are free to use whatever names you like...
Re: GSoC 2012 - Implement a few missing functions in D3DX9
Il 20 marzo 2012 19:12, Christian Costa titan.co...@gmail.com ha scritto: Le 20/03/2012 18:43, Józef Kucia a écrit : Hi there, I'm writing to discuss my project proposal for upcoming Google Summer of Code. I was recently playing with Wine, looking around in the code and searching through Wine bugzilla. As a result I found out that there is still a quite number of functions missing in Wine's D3DX implementation. However, some of them are probably not used much in a real software. For example, the mostly unimplemented ID3DXSkinInfo interface seems to be not used frequently. In this email I would like to present things which I would like to implement. I hope that implementation of this functions, or at least some of them, could improve Wine compatibility with a few real applications. Hi Józef, There are some work already done by Tony Wasserka which didn't get in git: http://repo.or.cz/w/wine/d3dx9TW.git Notice, though, that most of that code is not really suitable for inclusion in Wine, needing cleanup or more. I would like to implement the following things: * Implement D3DXCreateRenderToSurface function and ID3DXRenderToSurface interface. I think that's used quite often by applications and a good idea for GSoC. * Finish the implementation of ID3DXFont, particularly ID3DXFont::DrawTextA and ID3DXFont::DrawTextW functions. There is an implementation of it in the Tony Wasserka's repository Christian mentioned. Anyway, I don't know about the quality or the test coverage of that code, so I guess (re)implementing it may still be okay for GSoC. Having a different implementation to look at may even be a good thing, because otherwise implementing font drawing from scratch is not trivial. * Implement D3DXCreateCubeTextureFrom* and D3DXCreateVolumeTextureFrom* functions. That depends on the DDS file format support, I guess. * Add support for DDS file format in D3DXGetImageInfoFrom* functions. DDS support has been added in this patch: http://repo.or.cz/w/wine/d3dx9TW.git/commitdiff/f7fd4619de1cd0ea3bdf9b2fd0195c098d3b9bd0. I have an updated version and planned to submit it later. FOURCC support is also missing. That's interesting. Are you (Christian) still implementing it as a WIC decoder? I was under the impression that some DDS features (cube/volume textures I guess) aren't really supportable that way. I think I could also implement a few missing functions in ID3DXConstantTable interface, as a bonus task. That's fine too. This would probably require more effort about writing good tests and figuring out what the implementation needs to do than actually implementing the functions. I'm looking forward for a feedback. If you find any D3DX's function more important than functions enumerated by me, please let me know. I'm also looking for applications with heavy usage of D3DX dlls, especially applications which are using unimplemented or partially implemented functions (yes, I'm aware about http://wiki.winehq.org/DirectX-D3DX9). Hmm, that wiki page doesn't look too up to date. I'd look into bugzilla instead ( http://bugs.winehq.org/buglist.cgi?product=Winecomponent=directx-d3dx9resolution=--- for example). There is also some other D3DX9 patches lying around that didn't get in. You may take a look if you want to be sure not to make something more or less already done. Some duplication of effort is allowed under GSoC, so this is not a strict requirement. Also, if a patch was submitted but it is not in git, probably that means it wasn't good enough. That said, yeah, having two people independently implement the same thing at the same time is not the best. Cheers, Matteo.
Re: wpp: Stop parsing single and double quoted string constants after a first newline character.
Il 14 marzo 2012 23:02, Józef Kucia joseph.ku...@gmail.com ha scritto:
@@ -679,11 +649,23 @@ void pp_writestring(const char *format, ...)
}
}
pp_iqs,pp_dqs,pp_sqs\\. add_string(ppy_text, ppy_leng);
-pp_iqs,pp_dqs,pp_sqs\n {
+pp_iqs\n {
newline(1);
add_string(ppy_text, ppy_leng);
ppy_warning(Newline in string constant encountered (started
line %d), string_start());
}
+pp_dqs\n {
+ newline(-1);
+ add_string(ppy_text, ppy_leng);
+ ppy_warning(Newline in double quoted string constant
encountered (started line %d), string_start());
+ if(end_double_quoted_string()) return tDQSTRING;
+ }
+pp_sqs\n {
+ newline(-1);
+ add_string(ppy_text, ppy_leng);
+ ppy_warning(Newline in single quoted string constant
encountered (started line %d), string_start());
+ if(end_single_quoted_string()) return tSQSTRING;
+ }
/*
* Identifier scanning
I'm not sure this is legit. If I'm reading the C spec correctly, a
character or string constant can be only terminated by ' or
respectively. Although it can't contain unescaped newline characters
either, so it looks like we are actually in the realm of undefined
behavior.
So, I guess this change will only have any effect on non-standard
complying sources, and in the specific case of asm-style comments, it
brings us the behavior we want. On the other hand, it also actually
turns an error (unexpected end of file) into a warning, which may be
troublesome in general.
I hope it doesn't break anything else...
(You can take this email as okay for me, but someone else may know better)
Re: Preprocessing asm shaders
Il 12 marzo 2012 22:13, Józef Kucia joseph.ku...@gmail.com ha scritto: Hi, I'm writing because I found a bug in asm shader preprocessor in Wine and I'm not sure what's the best way to fix it. The D3DPreprocess function is an universal shader preprocess for HLSL and asm shaders. In fact, it's C-like language preprocessor and it doesn't understand assembler style comments (comments from ';' to the end of line). The fact that it doesn't understand these comments is a root of whole problem. Valid asm shaders can contain single or double quotation characters in assembler style comments, and these quotation characters will be treated as a beginning of quoted string constants while preprocessing. The Wine's implementation of D3DPreprocess behaves differently than Windows implementation in respect to strings passing the end of line. Generally, strings passing the end of line are an error in C, but they can result in a correct asm shader when contained in a assembler style comment. D3DPreprocess on Windows works fine for shaders which contains single or double quotation marks. It only issues an error in case of unmatched double quotation mark, but returns D3D_OK anyway. In Wine situation is different, wpp tries to find a matching quotation mark to the end of file, while shader preprocessor on Windows gives up on parsing a string when it gets a first newline character. The result of wpp's behavior is a truncated output and an error returned from D3DPreprocess. I wonder if it's fine to change wpp behavior for strings passing the end of line to mimic the behavior observed on Windows. Perhaps, strings passing the end of line could be treated as an error by default. Whereas D3DCompiler would activate an option to treat such strings as a warning. Thanks, Józef Kucia That is bug http://bugs.winehq.org/show_bug.cgi?id=24614. My shot at it was http://www.winehq.org/pipermail/wine-patches/2011-February/098609.html. You can see that my approach was essentially what you are suggesting. Still, that's not a very nice thing to do probably (and indeed my patch wasn't accepted at the time ;). Rereading it now, I notice that the patch also contains some wpp cleanup which should have been in separate patches, but I don't think that it was the reason of the reject... Dan tried a slightly different solution a while later, but still no dice. I'm unsure how a correct solution would be. The tests included in my patch essentially show that this asm-style comments thing is a PITA even for native d3dcompiler, so probably there can't be a really clean and nice solution. Also, some ad-hoc changes to wpp are needed anyway and have to kick in only when wpp is used by d3dcompiler, without affecting the other users. I'll think about it again as soon as I'll get some time. If you get any idea, I'd like to hear it (or, writing a good fix would be even better ...)
Re: GSoC 2011 - introduction
Il 09 marzo 2012 23:41, Józef Kucia joseph.ku...@gmail.com ha scritto: Hi, My name is Józef Kucia. I'm a Masters student in Computer Science at Wrocław University of Technology in Poland. I would like to apply for Google Summer of Code this year. I have a good knowledge of C and OpenGL. I also have limited experience with Direct3D and win32 programming. With regard to the Wine project, I'm familiar with the project from user perspective. I used to run some applications in Wine on my Gentoo Linux box, mostly games to be exact. Unfortunately, I haven't made any previous contributions to the Wine project, except submitting two small patches recently. I have read your project ideas for GSoC, and I am primarily interested in projects related to Direct3D. Nevertheless, I'm open to other projects. I would appreciate any suggestions or comments. Thanks, Józef Kucia Hi Józef, and welcome! Don't worry about not having contributed to Wine in the past, it's not important. Also, the patches you sent in the last few days make perfectly sense to me i.e. it shows you understand the code. About the project... that's the hard part. As Stefan wrote to another student interested in 3D-related GSOC projects, most of the missing stuff in wined3d is either too small or too big/hard for GSOC. We typically suggest to newcomers to just pick a game of their choice and make it run correctly on Wine, but that's not going to work for GSOC, essentially because you don't know how difficult it would be until you do it, while accountability is an important prerequisite for a project. So, while you can still try to follow this route, you'd need to figure out NOW what you'll be going to do during the project. Maybe implementing some easy pieces of d3d9ex or d3d10 (assuming there really are easy pieces to be done, I haven't really looked) is a possible alternative, but, again, hard to say. May be simpler to make a project out of d3dx9. There are still quite a few functions/object without an implementation and, at this point, a few of them (I'm thinking about mesh, surface and font functions, for example) are pretty much self contained. Probably you could choose an interesting subset of those for a project. Anyway, I'd suggest to look around in the code and/or Wine bugzilla and see if you find something interesting. Then just post a proposal of your own, we can adjust it if it looks too small or too big (it may very well happen that something which seems simple at first sight actually isn't, or even the other way around). You can also come by #winehackers on freenode, to further discuss this.
Re: [PATCH 1/5] wined3d: Clamp fog coordinate in the vertex shader.
Il 08 marzo 2012 22:10, Stefan Dösinger stefandoesin...@gmx.at ha scritto: Am Donnerstag, 8. März 2012, 18:22:15 schrieb Matteo Bruni: - shader_addline(buffer, gl_FogFragCoord = OUT[%u].%c;\n, i, reg_mask[1]); + shader_addline(buffer, gl_FogFragCoord = clamp(OUT[%u].%c, 0.0, 1.0);\n, i, reg_mask[1]); Is it correct to clamp the fog coord in the vertex shader, or should this be done in the pixel shader? The tests do not answer this question because they write the same fog coordinate for each vertex. Imagine for example two lines. In each of those lines, one vertex has oFog = 0.0. The other vertex has oFog = 1.1 in one line, and oFog = 100 in the other. If the value is clamped per vertex, those lines will look the same. If it's clamped per pixel, the latter line will have much steeper gradient I think I wrote a test at some point which shows that it is actually per-vertex. But I may be mistaken and such a test should be in the testsuite anyway, so I'll add it. Also, what happens to shaders that do not write oFog, but use oPos.z, especially in the situations where the near and far clipping planes are disabled? That's another good point, I don't think I tested that (that case isn't used by either Soul Reaver or the Sands of Time, the two games I was looking into at the time). I'll update the patches accordingly.
Re: OpenGL child windows (SketchUp)
Il 08 febbraio 2012 18:19, Brian Bloniarz brian.bloni...@gmail.com ha scritto:
Hi all,
I could use a little help fixing a window lag and screen corruption
issue in SketchUp:
http://bugs.winehq.org/show_bug.cgi?id=25912
Long story short, I think the implementation of OpenGL child rendering
needs a small update.
As a reminder, this is how an OpenGL child window is setup:
static BOOL set_win_format( HWND hwnd, XID fbconfig_id ) {
gl_drawable = XCreateWindow(...); // offscreen window
XCompositeRedirectWindow(..., gl_drawable, CompositeRedirectManual);
}
Here XComposite extension is render into an offscreen window.
When it's time to display, the contents of that window are copied into
the visible onscreen parent window (physDev is the onscreen window,
gl_drawable is the offscreen window):
BOOL X11DRV_SwapBuffers(PHYSDEV dev)
{
...
glXSwapBuffers(gdi_display, gl_drawable);
...
/* The GL drawable may be lagged behind if we don't flush first, so
* flush the display make sure we copy up-to-date data */
XFlush(gdi_display);
XCopyArea(gdi_display, gl_drawable, physDev...);
}
The XFlush() is the problem; the buffer swap must happen
before the copy, and accelerated renderers wouldn't guarantee to do
that on an X queue flush.
So how do you wait for a buffer swap? I wrote a test program,
can you say undefined behavior? I tested on a few drivers:
1) ATI fglrx binary driver:
glXSwapBuffers is instantaneous, existing code works fine.
2) NVidia binary driver:
glXSwapBuffers is asynchronous, neither XFlush() nor XSync()
is enough. Calling glXWaitGL or glFinish works.
3) Mesa DRI driver (Intel i915):
glXSwapBuffers is asynchronous. None of
XFlush/XSync/glXWaitX/glXWaitGL is enough. glFinish works.
4) r600g (Open Source accelerated ATI) driver:
glXSwapBuffers is asynchronous. None of the flush calls
wait for it (XFlush/XSync/glXWaitGL/glXWaitX/glFinish).
The last one is the tricky part -- this driver is also
based on the DRI, so I'm guessing it may appear on other
open-source accelerated drivers (looking at
http://www.x.org/releases/current/doc/dri2proto/dri2proto.txt
suggests that this is new behavior as of 1.99.2)
So how to come up with a common fix? I could think of 3
possibilities:
1) Use the GLX_OML_sync_control extension, it has an explicit
call to wait for a buffer swap. Not supported by NV or fglrx,
but we could fallback to glXWaitGL on those drivers. Probably
simplest.
2) Use the X Damage extension, wait for an XDamageNotify
event before copying. This should work everywhere, all
drivers probably implement damage so that compositing window
managers can use it.
3) Talk to the DRI people, maybe glXWaitGL should't return
when there's a pending a buffer swap. The GLX spec seems to
say it should wait, but it's a grey area and the Composite extension
was designed much later. Not a quick fix.
Any ideas or thoughts? If I hear nothing, I can code up a patch
that does (1), but it'd be great to hear from some people who
know this area well.
Thanks,
Brian Bloniarz
Nice analysis.
I'm not really such an expert in the area, but I think both (1) and
(2) make sense. I'm not sure in (1) case about the glXWaitGL call, as
technically glXSwapBuffers is not a GL call, but given that it works
for you (while XSync doesn't) on Nvidia, I guess it's fine... Make
sure it works correctly on all the drivers you mentioned, or even that
e.g. it doesn't cause excessive slowdowns.
Re: [PATCH 3/3] d3d9/tests: More D3DTSS_TEXTURETRANSFORMFLAGS projection tests.
2012/1/14 Saulius Krasuckas sauli...@ar.fi.lt: * On Tue, 27 Dec 2011, Matteo Bruni wrote: --- dlls/d3d9/tests/visual.c | 388 -- 1 files changed, 269 insertions(+), 119 deletions(-) Hello Matteo, this is http://source.winehq.org/git/wine.git/commitdiff/8dee7989f242b8ea624abc3b1fe929494d1fd329 The patch removed 6 failures on XP on real hardware (nVidia FX5200): http://test.winehq.org/data/18c20964e1e22f9aa974cc774bf77da6fc013716/xp_s2-sp2-nosound/d3d9:visual.html visual.c:4057: Test failed: proj: Pixel 162/122 has color 0x, expected 0x00FF visual.c:4062: Test failed: proj: Pixel 158/178 has color 0x, expected 0x00FF visual.c:4069: Test failed: proj: Pixel 318/118 has color 0x00ff, expected 0x visual.c:4071: Test failed: proj: Pixel 322/118 has color 0x00ff, expected 0x visual.c:4075: Test failed: proj: Pixel 322/122 has color 0x00ff, expected 0x visual.c:4078: Test failed: proj: Pixel 318/178 has color 0x, expected 0x00FF and introduced 14 new ones: http://test.winehq.org/data/277361d7be206e1ff7634998f0d750f7c6d66498/xp_s2-sp2-nosound/d3d9:visual.html visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (401, 361) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (401, 419) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (481, 359) has color 00ff, expected visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (481, 361) has color 00ff, expected visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (479, 359) has color 00ff, expected visual.c:3729: Test failed: D3DTTFF_COUNT3 | D3DTTFF_PROJECTED - bottom right: Pixel (479, 419) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_PROJECTED (like COUNT3 | PROJECTED, texcoord has only 3 components) - bottom right: Pixel (401, 361) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_PROJECTED (like COUNT3 | PROJECTED, texcoord has only 3 components) - bottom right: Pixel (401, 419) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_PROJECTED (like COUNT3 | PROJECTED, texcoord has only 3 components) - bottom right: Pixel (479, 361) has color , expected 00ff visual.c:3729: Test failed: D3DTTFF_PROJECTED (like COUNT3 | PROJECTED, texcoord has only 3 components) - bottom right: Pixel (479, 419) has color , expected 00ff visual.c:3729: Test failed: 0x (like COUNT3 | PROJECTED, texcoord has only 3 components) - top left: Pixel (81, 121) has color , expected 00ff visual.c:3729: Test failed: 0x (like COUNT3 | PROJECTED, texcoord has only 3 components) - top left: Pixel (81, 179) has color , expected 00ff visual.c:3729: Test failed: 0x (like COUNT3 | PROJECTED, texcoord has only 3 components) - top left: Pixel (159, 121) has color , expected 00ff visual.c:3729: Test failed: 0x (like COUNT3 | PROJECTED, texcoord has only 3 components) - top left: Pixel (159, 179) has color , expected 00ff The same machine running Win7 didn't exhibit any changes: http://test.winehq.org/data/18c20964e1e22f9aa974cc774bf77da6fc013716/win7_s2-enterprise-32-VAS/d3d9:visual.html http://test.winehq.org/data/277361d7be206e1ff7634998f0d750f7c6d66498/win7_s2-enterprise-32-VAS/d3d9:visual.html This probably has something to do with older nVidia driver version being used on Win7 - v96.85 - than the version used on XP - v163.75 . But on Win7 somewhat similar change is seen on virtual machine from Francois, only the failure count increases by 28 here: http://test.winehq.org/data/18c20964e1e22f9aa974cc774bf77da6fc013716/win7_fg-win7u64-1spie9-ja/d3d9:visual.html http://test.winehq.org/data/277361d7be206e1ff7634998f0d750f7c6d66498/win7_fg-win7u64-1spie9-ja/d3d9:visual.html Would you mind fixing this please:)? S. I somehow expected some test failures, since this is really testing invalid d3d9 states combinations. Those results show that there are some minor discrepancies with the actual output on the screen, but the bottom line is that invalid settings still work, which was the main objective of the tests. So adding some broken() around is probably okay. I'll send a patch. So, I was more or less expecting this and I did already notice the test failures, but thanks for the nice summary and heads up ;) I think I'll just ignore the virtual machine test failures: too many tests are failing already and I don't think those results are interesting anyway.
Re: po: Update Italian translation
2012/1/4 Luca Bennati luc...@gmail.com: #: winmm.rc:34 -#, fuzzy msgid There is no driver installed on your system! -msgstr Non è stato installato nessun driver nel sistema !\n +msgstr Non è presente nessun driver installato nel sistema! Hi Luca, While your new translation is clearer than the previous one, I think in the new translation presente and installato mean more or less the same thing i.e. having both is redundant IMO. How about something like Nessun driver è installato nel sistema! (or presente, or attualmente installato... you got it) instead?
Re: [PATCH 2/4] d3dx9: Implement ID3DXBaseEffect::SetVector().
Hi Rico,
2011/12/13 Rico Schüller kgbric...@web.de:
---
dlls/d3dx9_36/effect.c | 52
+--
1 files changed, 49 insertions(+), 3 deletions(-)
+static void set_vector(struct d3dx_parameter *param, CONST D3DXVECTOR4
*vector)
+{
+set_number((float *)param-data, param-type, vector, D3DXPT_FLOAT);
+if (param-columns 1) set_number((FLOAT *)param-data + 1,
param-type, (FLOAT *)vector + 1, D3DXPT_FLOAT);
+if (param-columns 2) set_number((FLOAT *)param-data + 2,
param-type, (FLOAT *)vector + 2, D3DXPT_FLOAT);
+if (param-columns 3) set_number((FLOAT *)param-data + 3,
param-type, (FLOAT *)vector + 3, D3DXPT_FLOAT);
+}
Can't you use a for loop instead? While you're at it, you should also
pick one between float and FLOAT.
Re: [PATCH 4/4] d3dx9: Implement ID3DXBaseEffect::SetValue().
2011/12/13 Rico Schüller kgbric...@web.de: --- dlls/d3dx9_36/effect.c | 42 -- 1 files changed, 40 insertions(+), 2 deletions(-) +TRACE(Copy %u bytes\n, param-bytes); +memcpy(param-data, data, bytes); That trace is not correct.
Re: [PATCH 2/5] d3d10: Implement D3D10StateBlockMaskDisableCapture().
2011/11/15 Henri Verbeet hverb...@codeweavers.com: --- +UINT end = start_idx + count; + start_idx += 7; + memset(field[start_idx 3], 0, (end 3) - (start_idx 3)); Isn't that dangerous if count is 7 (assuming that is a valid value)?
Re: d3dx9_36/tests [patch 2/2, try 3]: Add tests for D3DXCreatePolygon
2011/11/9 David Adam david.adam.c...@gmail.com:
Better, but there are still issues:
+if( polygon ) polygon-lpVtbl-Release(polygon);
+if( ppBuffer ) ID3DXBuffer_Release(ppBuffer);
You aren't setting those pointers to NULL (and your test shows that
D3DXCreatePolygon doesn't touch them when the call fails), so those
ifs actually are protecting nothing.
In patch 1/2, LockVertex/IndexBuffer should not fail in normal
circumstances, so it's probably better to use ERR instead of TRACE
there.
Also, please, try to fixup the code style. E.g. the preferred style
for 'if's and loops is to have a whitespace between the keyword and
the '(' and no whitespace between the parentheses and the condition.
Stick with that. Don't use hungarian notation (ppBuffer ...), fix
whitespaces around operators, etc
Re: [PATCH 3/3] wined3d: Report all FBO-attachable formats as suitable for render target usage.
2011/11/9 Matteo Bruni mbr...@codeweavers.com: --- This patch is not correct, please ignore it...
Re: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon
2011/11/7 David Adam david.adam.c...@gmail.com: Hello, any problem with this patch http://source.winehq.org/patches/data/80433 and this one http://source.winehq.org/patches/data/80434 Thanks in advance David -- Forwarded message -- From: David Adam david.adam.c...@gmail.com Date: 2011/10/30 Subject: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon To: wine-patches wine-patc...@winehq.org Fix a possible crash when calling D3DXCreateMeshFVF. A+ David Hello David, I see Rico preceded me (his points are very much valid too, and I'm going to overlap a bit), but as I have already written a reply anyway... I think your patches generally look somewhat ugly. Many of the comments I gave in http://www.winehq.org/pipermail/wine-devel/2011-March/089173.html for another patch of yours apply here all the same. About specific issues I have noticed here: Patch 1: +hr = D3DXCreateMeshFVF((DWORD)sides, (DWORD)(sides + 1), D3DXMESH_MANAGED, D3DFVF_XYZ | D3DFVF_NORMAL, device, polygon); Why those casts? +vertice = HeapAlloc(GetProcessHeap(), 0, 2 * (sides + 1) * sizeof(D3DXVECTOR3)); ... +memcpy(data, vertice, 2 * (sides + 1) * sizeof(D3DXVECTOR3)); What's the point in allocating and filling this additional array? You should just write into the vertex buffer. Same with the index buffer. +(polygon)-lpVtbl-UnlockVertexBuffer(polygon); those parentheses around polygon are useless. Patch 2: +hr = D3DXCreateBuffer(11 * sizeof(DWORD), ppBuffer); Why are you creating a D3DXBuffer here? It is probably created and returned by D3DXCreatePolygon (and indeed your own implementation in patch 1 does exactly that). If you want to have the cleanup work even when the tests fail, you have to protect the Release() calls somehow (e.g. initializing the object pointers to NULL and checking for != NULL on release). +index = *( (WORD*)( data + ( 3 * i + j ) * sizeof(WORD) ) ); You could make that much simpler by introducing a WORD pointer. Similarly for the vertex data. I guess the objection about expecting a specific vertex/index ordering is moot as it was in D3DXCreateBox case (i.e. there are applications blindly modifying the mesh), right?
Re: [1/2] d3dx9: Implemented non-equal declaration support in CloneMesh.
Hi Michael,
+for (i = 0; declaration1[i].Stream != 0xff; i++)
+{
+if (memcmp(declaration1[i], declaration2[i], sizeof(*declaration1)))
Doesn't that loop need to stop at the first end marker of either
declaration? I.e. may the first vertex declaration be longer than the
second one?
Re: [1/2] d3dx9: Implemented non-equal declaration support in CloneMesh.
2011/10/24 Michael Mc Donnell mich...@mcdonnell.dk:
On Mon, Oct 24, 2011 at 9:13 AM, Matteo Bruni matteo.myst...@gmail.com
wrote:
Hi Michael,
+ for (i = 0; declaration1[i].Stream != 0xff; i++)
+ {
+ if (memcmp(declaration1[i], declaration2[i],
sizeof(*declaration1)))
Doesn't that loop need to stop at the first end marker of either
declaration? I.e. may the first vertex declaration be longer than the
second one?
The first vertex declaration may be longer than the second one, e.g.
to make room for more vertex attributes. The loop actually still
breaks if the second declaration is shorter because it will have an
end marker and the first one doesn't.
I must admit it is difficult to read. I guess It would be easier to
read the code if it first finds the size of each declaration and does
a full memcmp if they're equal:
static BOOL declaration_equals(CONST D3DVERTEXELEMENT9 *declaration1,
CONST D3DVERTEXELEMENT9 *declaration2)
{
UINT size1, size2;
/* Find the size of each declaration */
for (size1 = 0; declaration1[size1].Stream != 0xff; size1++);
for (size2 = 0; declaration2[size2].Stream != 0xff; size2++);
/* If not same size then they are definately not equal */
if (size1 != size2)
return FALSE;
/* Check that all components are the same */
if (memcmp(declaration1, declaration2, size1*sizeof(*declaration1)) == 0)
return TRUE;
return FALSE;
}
I'll update the patch and send it again.
You're right, sorry for the noise. I see that you have already resent
the patches, that's fine anyway.
Re: wined3d: Report every texture format as filterable and blendable.
2011/10/12 Dan Kegel d...@kegel.com: Fails d3d9's visual tests here. os: Linux 3.0.0-12-generic, Ubuntu 11.10 gpu: GeForce GT 240/PCI/SSE2 3.3.0 NVIDIA 280.13 On Wed, Oct 12, 2011 at 2:52 PM, build...@kegel.com wrote: This is an experimental automated build and test service. Please feel free to ignore this email while we work the kinks out. For more info about this message, see http://wiki.winehq.org/BuildBot The Buildbot has detected a failed build on builder runtests-default while building Wine. Full details are available at: http://buildbot.kegel.com/builders/runtests-default/builds/273 (though maybe not for long, as I'm still reinstalling the buildbot periodically while experimenting) BUILD FAILED: failed shell_3 Errors: alarum: failed command was ../../../wine d3d9_test.exe.so visual.c fixme:win:EnumDisplayDevicesW ((null),0,0x33f0cc,0x), stub! fixme:d3d_draw:drawPrimitive Using software emulation because manual fog coordinates are provided fixme:d3d:state_shademode WINED3DSHADE_PHONG isn't supported fixme:d3d:state_shademode WINED3DSHADE_PHONG isn't supported visual.c:9147: Tests skipped: D3DFMT_G16R16 textures not supported as render targets. visual.c:9239: Test failed: Offscreen failed for D3DFMT_R16F: Got color 0x20, expected 0x18. visual.c:9239: Test failed: Offscreen failed for D3DFMT_G16R16F: Got color 0x2010ff, expected 0x1818ff. visual.c:9239: Test failed: Offscreen failed for D3DFMT_R32F: Got color 0x20, expected 0x18. visual.c:9239: Test failed: Offscreen failed for D3DFMT_G32R32F: Got color 0x2010ff, expected 0x1818ff. visual.c:9239: Test failed: Offscreen failed for D3DFMT_A32B32G32R32F: Got color 0x201000, expected 0x181800. visual.c:7741: Tests skipped: Card has unconditional pow2 support, skipping conditional NP2 tests make: *** [visual.ok] Error 5 * Call to xpconnect wrapped JSObject produced this error: * * Call to xpconnect wrapped JSObject produced this error: * GnuTLS error: GnuTLS internal error. Interesting. I didn't test on your kind of GPUs but I understood that it should support the feature I'm enabling. I'm not sure right now if it really doesn't support it or it is a driver bug of some kind. Thank you for catching it, if you have Windows installed on that machine I'll probably ask you to run a small testcase soon...
Re: d3dx9: CloneMesh test and improvements
2011/8/12 Octavian Voicu octavian.vo...@gmail.com: On Fri, Aug 12, 2011 at 12:59 PM, Michael Mc Donnell mich...@mcdonnell.dk wrote: Is it ok to use roundf and rintf? They're both C99 functions. Hello, As far as I know C99 is not allowed. However, you can emulate round by doing: floorf(val + 0.5f) According to [1] floorf is C99 (only floor is C89), but including math.h in wine actually gives you msvcrt's math.h which is a bit different and doesn't even have round nor rint. Gdiplus uses floorf [2] so it should be OK. No, we are still using native math library in wine dlls (actually, linking wine dlls to msvcrt is usually frowned upon), so those floorf calls go directly to libm. Anyway, if you look into wine msvcrt's code, floorf doesn't do anything else than directly calling native floorf (and without any #ifdef guard, as far as I can see), so it wouldn't really make a difference. That said, I don't know what is allowed and what's not. Requiring rintf or roundf support doesn't seem really asking more than what is needed now, but you could argue that we shouldn't use floorf in wine either. BTW, as you are actually rounding to convert a float to an integer type (as opposed to needing a rounded value still in a floating point variable), you can also avoid using floorf() altogether. So, e.g., instead of +dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); something like +dst_ptr[0] = src-x 0.0f ? (SHORT)(src-x * SHRT_MAX - 0.5f) :(SHORT)(src-x * SHRT_MAX + 0.5f); still works, since C floating point to integer conversion rules mandate for truncation. Octavian [1] http://linux.die.net/man/3/floorf [2] http://source.winehq.org/git/wine.git/?a=searchh=HEADst=greps=floorf
Re: d3dx9: CloneMesh test and improvements
2011/8/11 Michael Mc Donnell mich...@mcdonnell.dk: +dst_ptr[0] = src-x 0.0f ? (SHORT)ceilf(src-x * SHRT_MAX + 0.5f) :(SHORT)floorf(src-x * SHRT_MAX + 0.5f); You can use roundf() instead. Actually, notice that maybe what you actually need for correct rounding is rintf() (which essentially matches what D3DXFloat32To16Array does). You should probably test this (even on your own, not necessarily adding those to the testsuite, I'd say). Still on the conversion: going on with this strategy is going to require a ton of code to handle every src-dst format combination. You may look into doing that in two steps: first converting src to float4, then float4 into dst. I haven't looked into it, so there may be some detail making this impractical, but still it's worth a try, I guess.
Re: d3dx9: CloneMesh test and improvements
2011/8/8 Michael Mc Donnell mich...@mcdonnell.dk:
Hi
I've been working on a test and improvements for CloneMesh as part of
GSoC 2011. I would be grateful if anyone could comment on my work.
There are five test cases in the attached patch:
0. Basic mesh cloning. Declaration has position and normal, and the
new declaration is the same as the original.
1. Same as test 0, but with 16-bit indices.
2. Shows that the vertex buffer can be widened so that it has room
for a new vertex component. The declaration has position and normal,
and the new declaration has texture coordinates after position and
normal.
3. Same as test 2, but the new declaration has the texture
coordinates in between position and normal.
4. Shows that a vertex component can be converted into another type
if the new declaration specifies another type. The declaration has
position and normal. The normal is a FLOAT2 and in the new declaration
it is instead FLOAT16_2.
Test 0 and 1 tests what is already known to work in the current
implementation. Test 2, 3 and 4 do not work in the current
implementation because the original and new declarations are not the
same.
I've also improved the current CloneMesh implementation so that it
also passes test 2, 3, and 4. The patch is not complete because it
does not support converting all the possible types. I expect to
implement conversion for the remaining types during this week.
Again any comments or ideas are welcome.
Thanks,
Michael Mc Donnell
Hello Michael,
I have some comments:
+const UINT d3dx_decltype_size[D3DDECLTYPE_UNUSED] =
+{
I don't really like sizing the vector like that. Just remove the
D3DDECLTYPE_UNUSED between the square brackets, I'd say. Otherwise,
you could use the D3DMAXDECLTYPE define.
+ /* D3DDECLTYPE_FLOAT1*/ 1 * 4,
...
You may want to replace the 4 with a sizeof(float) for better clarity.
Same for the others.
+if (orig_declaration.Method == declaration[i].Method
+ orig_declaration.Usage == declaration[i].Usage
+ orig_declaration.UsageIndex == declaration[i].UsageIndex)
Does the Method field really need to be compared? Maybe add a test if
that is the case.
You may also e.g. test if in a POSITION, NORMAL, TEXCOORD(0) -
POSITION, NORMAL, TEXCOORD(1) CloneMesh (or even the other way around)
the texture coordinates are actually lost or not.
+for (i = 0; orig_declaration[i].Stream != 0xff; i++) {
+if (memcmp(orig_declaration[i], declaration[i],
sizeof(*declaration))) {
+same_declaration = FALSE;
+break;
+}
+}
+
+if (vertex_size != orig_vertex_size)
+same_declaration = FALSE;
Not really a big thing, but you could do the vertex size check first
and then compare the fields only if same_declaration is still TRUE.
The tests look good to me, except some nits like the ok() in
check_vertex_components in the D3DDECLTYPE_FLOAT1 case, splitting long
lines and such.
Re: wined3d: Update GLSL spam filter for recent fglrx versions.
2011/8/3 Stefan Dösinger stefandoesin...@gmx.at: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am 03.08.2011 um 17:31 schrieb Matteo Bruni: Users tend to get confused by those messages on the terminal. Since we already have code to filter irrelevant GLSL compiler messages, let's update it. I think the plan a while ago was to remove the spam filter and not print driver messages as FIXMEs(just as WARNs or TRACEs) when the compile was successful. Okay, I'll try something like that. -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJOOWwJAAoJEN0/YqbEcdMwRF8QAIMOF0XTEsl8+DSmBsuDzIX4 dSjR96gAtHlQGaq11P4Auv2cX1slykcLQrfJWy4zPOCty97psJHQSzSmiEqiRyK2 7Eqa8iKvlqsXJhBQmkOWZM3XBIUIdsfsZrT+jo7jDOjnwF0CNZ7DP6ch3q01gWOw UWlNukHd2qZ29Awq5zuf6T9O45/4W1i85XPrWzLlzhAGeQ7dhTHzddpLfjO1Tb5B GxcmeSmyBP2RMEgjl6eXGk2mgz9OmjSPJrFTgaav0l5mCvhO/HcRczqhvtV08DDd gO3WCvxRywOwL78cOrudM1Aqd8o33etYK0hQGT3zw1kreXbFL7rPS/APs38ZF/53 m+3pRg2JtJiQQmWSjM4ou4H15H72Z8zoXMxAvvguL8YveuFNu2Q18SfLkNc9Qhwv sMtRkxM55ap0D8RAYw60eWORlPYBwMD8zF1lKz3xImIemoMflo7CSrvrWv7KUXJE 0IPOGpJyCtTIrShfh6Hvj53FwWb16FfphETnvwFffY7k42mkbwU+Sa76aZ9Wklnl DODFQ2VLgMtCyO/3he2b4rc2KAlc567HKJI2ND8hYXvQH8M2q6ebps5LrphglPnb 87e3jdBcNBISJ0t2/jamNRFj+uPY2UNoga+5xKwnC1uqKe1m9mmzfvm25SIAMBbE bihjIFyAg+Bv5Z6Lrzag =Qbf6 -END PGP SIGNATURE-
Re: [3/3] ddraw/tests: Add a couple of fog tests.
2011/7/27 Marvin test...@testbot.winehq.org: Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=12866 Your paranoid android. === WINEBUILD (build) === Make failed Apparently the testbot got confused and mixed up my two 3-patch series (in hindsight, I should have sent the two series with some delay in between).
Re: d3dx9_36: Use SUCCEEDED instead of !FAILED.
2011/7/19 Michael Stefaniuc mstef...@redhat.de: --- dlls/d3dx9_36/mesh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c index 94a291f..d8f72c4 100644 --- a/dlls/d3dx9_36/mesh.c +++ b/dlls/d3dx9_36/mesh.c @@ -3376,7 +3376,7 @@ HRESULT WINAPI D3DXLoadMeshFromXResource(HMODULE module, if (!resinfo) return D3DXERR_INVALIDDATA; hr = load_resource_into_memory(module, resinfo, buffer, size); - if (!FAILED(hr)) return D3DXERR_INVALIDDATA; + if (SUCCEEDED(hr)) return D3DXERR_INVALIDDATA; return D3DXLoadMeshFromXInMemory(buffer, size, options, device, adjacency, materials, effect_instances, num_materials, mesh); -- 1.7.6 Isn't that supposed to be if (FAILED(hr)) ? If I'm not mistaken, now it is returning failure when the resource is correctly loaded... Thumbs up for Michael's scripts finding real bugs ;)
Re: [PATCH 1/2] d3dx9_36: Added support for structs and arrays to ID3DXConstantTable (Try 2)
2011/7/14 Travis Athougies iamm...@gmail.com: Needed for Assassin's Creed Brotherhood and Dead Space 2. Changes from last patch: - New way of distinguishing handles from strings - Support for parent constants in ID3DXConstantTable::GetConstantByName and ID3DXConstantTable::GetConstant - Implemented ID3DXContantTable::GetConstantElement You should split the reworking of D3DXHANDLEs into a separate patch. GetConstantElement could be on its own patch too, probably.
Re: [PATCH 1/3] d3dx9/line: Implemented ID3DXLine's Draw method.
2011/6/30 Charles Welton charles...@gmail.com: --- +memcpy(buffer_mem, vertices, size); There is no need to allocate and write into your own buffer then copy the content, you can directly write into the vertex buffer. A more important issue: MSDN says The ID3DXLine interface implements line drawing using textured triangles, which is probably true (as far as I know there is otherwise no support in D3D9 to antialiased/stippled/wide lines, which are instead supported by ID3DXLine). So, drawing with D3DPT_LINESTRIP primitives in general is probably not the right way...
Re: [2/2] d3dx9_36: Implement and add tests for D3DXFloat16To32Array.
2011/6/29 Misha Koshelev misha...@gmail.com: --- ... +else return sgn * powf(2, -14.0f) * ((float)m / 1024.0f); ... +return sgn * powf(2, (float)e - 15.0f) * (1.0f + ((float)m / 1024.0f)); I think you don't need the explicit casts here, the values should be automatically promoted to floats as the other operand is a float as well.
Re: [PATCH 2/2] d3dx9/line: Implemented tests for ID3DXLine's Begin and End methods.
2011/6/18 Charles Welton charles...@gmail.com: --- dlls/d3dx9_36/tests/line.c | 23 +++ 1 files changed, 23 insertions(+), 0 deletions(-) +srand(time(NULL)); +random = ((FLOAT) rand())/RAND_MAX; /* Random value between 0.0 and 1.0. We're using this later. */ I didn't check why your patch failed with the testbot. Nevertheless, you should avoid to use random values: the tests should be repeatable. Just pick some arbitrary values and use them. Also, for better clarity, you may want to use a different variable for the modified identity matrix (because, at that point, it isn't identity anymore). Maybe you could check the world matrix between ID3DXLine_Begin and ID3DXLine_End too.
