Re: [1/4] ddraw/tests: Add a test for YV12 partial block locks and lock offsets

2011-11-08 Thread Marvin
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=15324

Your paranoid android.


=== WINEBUILD (build) ===
Patch failed to apply




Re: [4/4] ddraw: Set correct HEL and HAL color models

2011-11-08 Thread Marvin
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=15327

Your paranoid android.


=== WINEBUILD (build) ===
Patch failed to apply




Re: [3/4] ddraw: Add more tests and fixes for SetSurfaceDesc

2011-11-08 Thread Marvin
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=15326

Your paranoid android.


=== WINEBUILD (build) ===
Patch failed to apply




Re: [PATCH 1/5] wined3d: Don't print FIXMEs for misaligned surface maps.

2011-11-08 Thread Henri Verbeet
On 8 November 2011 21:33, Stefan Dösinger  wrote:
> On Tuesday 08 November 2011 20:48:55 Henri Verbeet wrote:
>> -                    FIXME("Partial block lock with %s\n",
>> debug_d3dpool(surface->resource.pool));
> I added this fixme because we have to tests for the offset that is returned in
> this case. Do you have an application that triggers this fixme, or do you want
> to suppress the fixmes in the tests?
>
Mostly because of the spam this introduces in the tests. Arguably this
is something the tests you added should have covered, but I don't
think it's a particularly interesting case and what we currently
return is probably the only offset that makes some kind of sense
anyway.




Re: [PATCH 1/5] wined3d: Don't print FIXMEs for misaligned surface maps.

2011-11-08 Thread Stefan Dösinger
On Tuesday 08 November 2011 21:33:39 Stefan Dösinger wrote:
> I added this fixme because we have to tests for the offset that is returned
> in this case.
Sorry, typo: I meant "because we have no tests".


signature.asc
Description: This is a digitally signed message part.



Re: [PATCH 1/5] wined3d: Don't print FIXMEs for misaligned surface maps.

2011-11-08 Thread Stefan Dösinger
On Tuesday 08 November 2011 20:48:55 Henri Verbeet wrote:
> -FIXME("Partial block lock with %s\n",
> debug_d3dpool(surface->resource.pool));
I added this fixme because we have to tests for the offset that is returned in 
this case. Do you have an application that triggers this fixme, or do you want 
to suppress the fixmes in the tests?


signature.asc
Description: This is a digitally signed message part.



Re: Adopt a patch?

2011-11-08 Thread Frédéric Delanoy
On Tue, Nov 8, 2011 at 19:41, Michael Ost  wrote:
> I have a patch for a bug that I won't be able to shepherd through the patch
> submission process for some time. The bug is that GetFileVersionInfoSize
> doesn't work for files with path names longer than 128 characters.
>
> The patch includes an untried unit test. And I've confirmed the behavior in
> Windows Vista.
>
> We use an older version of Wine, and my boss won't let me take the time
> right now to rework the patch for the newest Wine sources. If someone wants
> to they can take this patch, tweak it for the new wine, update it to
> wine-patch requirements and push it through the system.
>
> Otherwise I'll submit as soon as I can.

You should probably create a new/update an existing bug in wine
bugzilla and attach your patch there.

Frédéric




Adopt a patch?

2011-11-08 Thread Michael Ost
I have a patch for a bug that I won't be able to shepherd through the 
patch submission process for some time. The bug is that 
GetFileVersionInfoSize doesn't work for files with path names longer 
than 128 characters.


The patch includes an untried unit test. And I've confirmed the behavior 
in Windows Vista.


We use an older version of Wine, and my boss won't let me take the time 
right now to rework the patch for the newest Wine sources. If someone 
wants to they can take this patch, tweak it for the new wine, update it 
to wine-patch requirements and push it through the system.


Otherwise I'll submit as soon as I can.

Cheers,

Michael Ost
--- wine-1.1.7/dlls/kernel32/file.c.RPM	2011-11-02 14:04:34.0 -0700
+++ wine-1.1.7/dlls/kernel32/file.c	2011-11-02 14:06:42.0 -0700
@@ -2476,8 +2476,29 @@
 {
 /* Now look for the file */
 
-if (!SearchPathA( NULL, name, NULL, sizeof(ofs->szPathName), ofs->szPathName, NULL ))
-goto error;
+#if 1 // todo
+		CHAR pathName[MAX_PATH];
+		DWORD len;
+
+		len = SearchPathA( NULL, name, NULL, MAX_PATH, pathName, NULL );
+		if (len == 0)
+			goto error;
+		lstrcpynA(ofs->szPathName, pathName, sizeof(ofs->szPathName));
+// todo: copy as much as you can into ofs ... and confirm this in windows
+
+		TRACE("found %s\n", debugstr_a(pathName) );
+
+		if (mode & OF_DELETE)
+		{
+			if (!DeleteFileA( pathName )) goto error;
+			TRACE("(%s): OF_DELETE return = OK\n", name);
+			return TRUE;
+		}
+
+		handle = LongToHandle(_lopen( pathName, mode ));
+#else
+		if (!SearchPathA( NULL, name, NULL, sizeof(ofs->szPathName), ofs->szPathName, NULL ))
+			goto error;
 
 TRACE("found %s\n", debugstr_a(ofs->szPathName) );
 
@@ -2489,6 +2510,7 @@
 }
 
 handle = LongToHandle(_lopen( ofs->szPathName, mode ));
+#endif
 if (handle == INVALID_HANDLE_VALUE) goto error;
 
 GetFileTime( handle, NULL, NULL, &filetime );
--- wine-1.1.7/dlls/kernel32/file16.c.RPM	2011-11-02 14:10:28.0 -0700
+++ wine-1.1.7/dlls/kernel32/file16.c	2011-11-02 15:30:13.0 -0700
@@ -122,6 +122,10 @@
 FILETIME filetime;
 WORD filedatetime[2];
 const char *p, *filename;
+#if 1 // todo
+	CHAR pathName[MAX_PATH];
+	DWORD len;
+#endif
 
 if (!ofs) return HFILE_ERROR;
 
@@ -194,11 +198,32 @@
 
 if (filename)
 {
-BOOL found;
+#if 1 // todo
 char *path = get_search_path();
 
 if (!path) goto error;
-found = SearchPathA( path, filename, NULL, sizeof(ofs->szPathName),
+			len = SearchPathA( path, filename, NULL, MAX_PATH, pathName, NULL );
+			HeapFree( GetProcessHeap(), 0, path );
+			if (len == 0) goto error;
+			lstrcpynA(ofs->szPathName, pathName, sizeof(ofs->szPathName));
+		}
+
+		TRACE("found %s\n", debugstr_a(pathName) );
+
+		if (mode & OF_DELETE)
+		{
+			if (!DeleteFileA( pathName )) goto error;
+			TRACE("(%s): OF_DELETE return = OK\n", name);
+			return 1;
+		}
+
+		handle = (HANDLE)_lopen( pathName, mode );
+#else
+			BOOL found;
+			char *path = get_search_path();
+
+			if (!path) goto error;
+			found = SearchPathA( path, filename, NULL, sizeof(ofs->szPathName),
  ofs->szPathName, NULL );
 HeapFree( GetProcessHeap(), 0, path );
 if (!found) goto error;
@@ -214,6 +239,7 @@
 }
 
 handle = (HANDLE)_lopen( ofs->szPathName, mode );
+#endif
 if (handle == INVALID_HANDLE_VALUE) goto error;
 
 GetFileTime( handle, NULL, NULL, &filetime );
--- wine-1.1.7/dlls/version/tests/info.c.RPM	2011-11-08 10:31:24.0 -0800
+++ wine-1.1.7/dlls/version/tests/info.c	2011-11-02 16:17:00.0 -0700
@@ -183,6 +183,19 @@
"Expected ERROR_RESOURCE_DATA_NOT_FOUND, got %d\n", GetLastError());
 
 DeleteFileA("test.txt");
+
+	static const char* const kVeryLongFileName = "c:\\a very very long and absolute pathname that is longer than the one hundred and twenty-eight character limit of the OFSTRUCT szPathName member";
+	create_file(kVeryLongFileName);
+	SetLastError(0xdeadbeef);
+	hdl = 0xcafe;
+	retval = GetFileVersionInfoSizeA(kVeryLongFileName, &hdl);
+	ok(retval == 0, "Expected 0, got %d\n", retval);
+	ok(hdl == 0, "Expected 0, got %d\n", hdl);
+	ok(GetLastError() == ERROR_RESOURCE_DATA_NOT_FOUND ||
+	   GetLastError() == ERROR_BAD_FORMAT || /* win9x */
+	   GetLastError() == ERROR_SUCCESS, /* win2k */
+	   "Expected ERROR_RESOURCE_DATA_NOT_FOUND, got %d\n", GetLastError());
+	DeleteFileA(kVeryLongFileName);
 }
 
 static void VersionDwordLong2String(DWORDLONG Version, LPSTR lpszVerString)



Re: apphelp: add a stub for ApphelpCheckShellObject

2011-11-08 Thread Dmitry Timoshkov
Austin English  wrote:

> +BOOL WINAPI ApphelpCheckShellObject(REFCLSID ObjectCLSID, BOOL 
> ShimIfNecessary, ULONGLONG *pullFlags)
> +{
> +FIXME("stub: %s %p\n", debugstr_guid(ObjectCLSID), pullFlags );
> +return TRUE;
> +}

It's a common convention in Wine to avoid hungarian notation by all means.
In this case naming variables something like 'clsid, shim, flags' woud be
better.

-- 
Dmitry.




Re: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon

2011-11-08 Thread Matteo Bruni
2011/11/7 David Adam :
> 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 
> Date: 2011/10/30
> Subject: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon
> To: wine-patches 
>
>
> 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: [10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.

2011-11-08 Thread Stefan Dösinger
On Tuesday 08 November 2011 12:08:21 Octavian Voicu wrote:
> It gets a bit harder when you have more than a couple of tests,
> because you have to follow the sequence of flashes, but the message
> from the failures should be descriptive enough.
I do that with things like if(i == 5) Sleep(99). If you want to see 
something you'll have to add a sleep anyway.

> >> of d3d, d3d2, d3d3, d3d7, where to have most of the things we store
> >> globally or in local vars now, and make the create/release functions
> >> work with that.
> > 
> > Yes, I was about to write something like that as a response to the
> > cleanup patches.
> 
> Is it OK to leave them like this for now? That kind of refactoring is
> going to make this patch series endless.
I think its better to have the cleanup done before adding the d3d2/3 tests and 
infrastructure. You don't have to send all of the patches at once. I usually 
don't send more than 5 patches a day. This makes it more likely all mails make 
it to wine-patches and keeps discussions of the patches focused.

http://wiki.winehq.org/SubmittingPatches has the general patch submission 
guidelines. The "Small, clear and atomic changes" part is the trickiest aspect 
of this. All patches have to compile and pass the tests, otherwise Alexandre 
doesn't commit them. Single patches should be as small as possible, but make 
sense on their own. If you split up patches, and patch 1 prepares things for 
patch 2 and isn't needed otherwise, send them in the same patch series.

In your patches you have a few separate things:
* Patch 1 makes sense on its own
* Patches 2 and 3 clean up variables. Introduction of a structure would fit 
into those
* Patches 4, 5 and 6 fix issues on vmware
* 7, 8 add d3d2 tests
* 9, 10 add d3d3 tests
* 11 adds a d3d7 test

The separation into small patches looks good. You can send more than one of 
those patch blocks at once, but you don't have to.

Don't be afraid to have a bigger set of unsent patches in your tree, git makes 
it fairly easy to work with them. For editing patches, git rebase --
interactive is very helpful. Branches can be very useful for bigger features 
or fixes you're working on. There are also other tools like stgit that are 
worth a look.

For example, I currently have 7 unsent patches in my tree that are ready to 
go. I have 3 branches with features that I've put on ice until after Wine 1.4 
and 4 tests and bugfixes that need some more work before I'll send them(They 
currently live as test patches on my git tree on the Windows partition). I 
also have a branch with hacks for personal use that aren't of any use for 
others and will never make it into Wine.


signature.asc
Description: This is a digitally signed message part.



Re: [01/11] ddraw/tests: Fix a couple of copy-paste typos in test messages.

2011-11-08 Thread Stefan Dösinger
This patch is OK, the others need some discussion and work.


signature.asc
Description: This is a digitally signed message part.



Re: [3/3] ddraw/tests: Add a partial block lock test

2011-11-08 Thread Alexandre Julliard
Stefan Dösinger  writes:

> On Tuesday 08 November 2011 13:45:20 Alexandre Julliard wrote:
>> Adding a range of years in a copyright notice is usually discouraged,
>> because it's only valid if you have actually made significant changes
>> every year, which doesn't seem to be the case here.
> What's the preferred way? Add separate lines for each year, or ranges?

Enumerate the years, like "Copyright 2007, 2009, 2011".

-- 
Alexandre Julliard
julli...@winehq.org




Re: [PATCH 3/7] ntoskrnl: intialze unknown fields of the irp with 0xce in IoInitializeIrp

2011-11-08 Thread Alexandre Julliard
Bernhard Loos  writes:

> @@ -864,6 +873,10 @@ VOID WINAPI IoCompleteRequest( IRP *irp, UCHAR 
> priority_boost )
>  
>  iosb = irp->UserIosb;
>  status = irp->IoStatus.u.Status;
> +
> +assert( status != STATUS_PENDING );
> +assert( status != 0xdeafbeef );
> +

You can't assert on things that are set by the caller.

-- 
Alexandre Julliard
julli...@winehq.org




Re: [3/3] ddraw/tests: Add a partial block lock test

2011-11-08 Thread Stefan Dösinger
On Tuesday 08 November 2011 13:45:20 Alexandre Julliard wrote:
> Adding a range of years in a copyright notice is usually discouraged,
> because it's only valid if you have actually made significant changes
> every year, which doesn't seem to be the case here.
What's the preferred way? Add separate lines for each year, or ranges?


signature.asc
Description: This is a digitally signed message part.



Re: kernel32/tests: add tests for GetDllDirectory (try 2)

2011-11-08 Thread Alexandre Julliard
Thomas Faber  writes:

> +/* no buffer, determine length */
> +SetLastError(0xdeadbeef);
> +ret = pGetDllDirectoryA(0, NULL);
> +error = GetLastError();
> +ok(ret == length + 1, "Expected %u, got %u\n", length + 1, ret);
> +ok(error == 0xdeadbeef, "Error is %u\n", error);

Testing last error on success is generally not useful.

-- 
Alexandre Julliard
julli...@winehq.org




Re: [3/3] ddraw/tests: Add a partial block lock test

2011-11-08 Thread Alexandre Julliard
Stefan Dösinger  writes:

> @@ -4,7 +4,7 @@
>   * Copyright (C) 2005 Antoine Chavasse (a.chava...@gmail.com)
>   * Copyright (C) 2005 Christian Costa
>   * Copyright 2005 Ivan Leo Puoti
> - * Copyright (C) 2007 Stefan Dösinger
> + * Copyright (C) 2007-2011 Stefan Dösinger for CodeWeavers

Adding a range of years in a copyright notice is usually discouraged,
because it's only valid if you have actually made significant changes
every year, which doesn't seem to be the case here.

-- 
Alexandre Julliard
julli...@winehq.org




Re: Fwd: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon

2011-11-08 Thread Rico Schüller

Hi David,

Impl:
- Probably you may unlock and free the polygon when allocating the 
vertice/index fail, why do you need the vertice at all? Couldn't you 
just write directly to "data", since you're holding the lock anyway?
- if locking fails, you're leaking the polygon memory, this happens for 
nearly all error checks

- the TRACE shouldn't contain "stub"
- there are some style issues

Test:
- it may be nice to test if polygon or ppBuffer are set to NULL on hr != 
D3D_OK or if they are untouched
- Why are you creating a buffer ppBuffer at all? From your 
implementation it looks like it is done when using adjacency? Also I'd 
expect, when D3DXCreateBuffer fails, ppBuffer is NULL and the release at 
the end label would crash, even if it still has uninitialized memory.
- there is an unlock for the vertexbuffer missing and you may check the 
return value from the unlock call

- there are some copy and paste mistakes
- I think a failing Lock*Buffer should also get a failing test, I 
wouldn't just print the skip.
- I'd check the count from all Release calls, that way, you'd see if you 
forgot to release something


Hope this helps
Rico

Am 07.11.2011 20:58, schrieb David Adam:

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
Date: 2011/10/30
Subject: d3dx9_36 [patch 1/2, resent]: Implement D3DXCreatePolygon
To: wine-patches


Fix a possible crash when calling D3DXCreateMeshFVF.

A+

David









Re: [10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.

2011-11-08 Thread Octavian Voicu
On Tue, Nov 8, 2011 at 12:18 PM, Stefan Dösinger  wrote:
> On Tuesday 08 November 2011 01:14:22 Octavian Voicu wrote:
>> The idea behind using triangles was to fit everything in one screen so
>> I could debug it visually very easily. I'm thinking I can still have
>> that if I draw full-screen quads in a loop, like you suggest, then
>> blit one small rectangle to the primary surface, in different
>> positions for each test.
> I don't think that's necessary, I found those one-quad screens easier to debug
> than multi-quads, although that may be a matter of taste.

It gets a bit harder when you have more than a couple of tests,
because you have to follow the sequence of flashes, but the message
from the failures should be descriptive enough.

>> I think
>> we should cover special error cases if there are any (that is, if
>> D3DERR_INVALIDVERTEXTYPE is not returned for D3DVT_ constants).
> Well, you can't test them. D3DVT_LVERTEX is 2, and D3DFVF_XYZ is 2. The number
> 2 doesn't have any tags attached, so what you'd be testing is how
> drawPrimitive responds to D3DFVF_RESERVED0, not what it does with D3DVT_*
> enums.
>
> The statement in the SDK doesn't make sense with that in mind.

I thought D3DVT_ values interpreted as FVFs didn't make sense. It
seems Windows accepts those values and returns D3D_OK, so I'll drop
those tests.

>  > About the ddraw/visual tests: I think we need one structure for each
>> of d3d, d3d2, d3d3, d3d7, where to have most of the things we store
>> globally or in local vars now, and make the create/release functions
>> work with that.
> Yes, I was about to write something like that as a response to the cleanup
> patches.

Is it OK to leave them like this for now? That kind of refactoring is
going to make this patch series endless.

Octavian




Re: [10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.

2011-11-08 Thread Octavian Voicu
[resent to include wine-devel]

On Tue, Nov 8, 2011 at 1:23 AM, Stefan Dösinger  wrote:
> Design-wise I think the 8 triangles and readback grid make the test fairly
> difficult to read. You could set up a structure that contains the d3d 
> parameters
> you're changing between the tests and the expected color and then loop over
> the table. Something like this:
>
> const struct
> {
>        BOOL d3drs_lighting;
>        DWORD dp_flags; <- for things like D3DP_DONOTLIGHT
>        DWORD fvf;
>        void *vertices;
>        DWORD expected_color;
> }
> test_data[] =
> {
>        ...
> };
>
> You can also think about sharing this control table between d3d2, d3d3 and
> d3d7 by having different color fields for that, but I don't know how well this
> will work.

Henry wasn't very happy about this either. Your fix sounds
interesting, I'm going to give it a try.

The idea behind using triangles was to fit everything in one screen so
I could debug it visually very easily. I'm thinking I can still have
that if I draw full-screen quads in a loop, like you suggest, then
blit one small rectangle to the primary surface, in different
positions for each test.

> On Monday 07 November 2011 23:27:13 Octavian Voicu wrote:
>> +    DWORD expected[] = { GRAY, WHITE, YELLOW2, YELLOW, GREEN, GREEN, BLUE,
> BLUE };
> Please make data const where you can.
Point taken.

>> +        /* Note: IDirect3DDevice3::DrawPrimitive calls with D3DVT_ vertex
>> types should fail. */ +
>> +        /* Triangle 0 -- D3DFVF_VERTEX (vertices with normals, but without
>> colors). */ +        hr = IDirect3DDevice3_DrawPrimitive(Direct3DDevice3,
>> D3DPT_TRIANGLELIST, D3DVT_VERTEX,
>> +                                            vertices, 3, D3DDP_WAIT);
>> +        ok(hr != DDERR_INVALIDPARAMS, "DrawPrimitive returned: %08x\n",
>> hr);
> I don't think there's a point in testing this, both D3DVT_* and D3DFVF_* are
> defines / enums that map to integers, and it is obvious that you get wrong
> behavior if you mix those.
>
> The ok condition looks strange. If you're trying to avoid a todo_wine, don't
> do that, just mark tests that show a difference between Wine and Windows as
> todo_wine.
> (I guess Wine currently returns DD_OK, and Windows returns
> D3DERR_INVALIDVERTEXFORMAT)

DirectX SDK 6.1 docs says that "If you attempt to use one of the
members of D3DVERTEXTYPE, the method fails, returning
DDERR_INVALIDPARAMS", so I wanted to test exactly that. I accidentally
wrote "!=" instead of "==" and forgot about it since Windows did not
complain (SDK is probably inaccurate), nor did Wine. Will look into it
and probably write some separate tests, outside the test loop. I think
we should cover special error cases if there are any (that is, if
D3DERR_INVALIDVERTEXTYPE is not returned for D3DVT_ constants).

>> +    /* Make sure getPixelColor works correctly. */
>> +    color = getPixelColor(device, 320, 240);
>> +    ok(color == RED, "getPixelColor returned: %08x\n", color);
> We have some sanity checks for that in the main function

I know that, but I figured it didn't hurt to have an extra check
there, since each D3D2, D3D3, and D3D7 test uses a different version
of the getPixelColor function.

> Most of this applies to the d3d2 and d3d3 tests as well. I haven't looked over
> the cleanup patches yet.

Thanks for the quick and thorough feedback, it was really helpful!

About the ddraw/visual tests: I think we need one structure for each
of d3d, d3d2, d3d3, d3d7, where to have most of the things we store
globally or in local vars now, and make the create/release functions
work with that. With my cleanup patches I think it will be easier to
refactor code to use such a structure. However, my patch stack was
getting uncomfortably large and I wanted to commit it, so I can get to
the good part -- fixing the game I'm debugging. I might look into such
a refactoring later.

Octavian




Re: [10/11] ddraw/tests: Add assorted D3D3 DrawPrimitive/visual tests.

2011-11-08 Thread Stefan Dösinger
On Tuesday 08 November 2011 01:14:22 Octavian Voicu wrote:
> The idea behind using triangles was to fit everything in one screen so
> I could debug it visually very easily. I'm thinking I can still have
> that if I draw full-screen quads in a loop, like you suggest, then
> blit one small rectangle to the primary surface, in different
> positions for each test.
I don't think that's necessary, I found those one-quad screens easier to debug 
than multi-quads, although that may be a matter of taste.

> I think
> we should cover special error cases if there are any (that is, if
> D3DERR_INVALIDVERTEXTYPE is not returned for D3DVT_ constants).
Well, you can't test them. D3DVT_LVERTEX is 2, and D3DFVF_XYZ is 2. The number 
2 doesn't have any tags attached, so what you'd be testing is how 
drawPrimitive responds to D3DFVF_RESERVED0, not what it does with D3DVT_* 
enums.

The statement in the SDK doesn't make sense with that in mind.

 > About the ddraw/visual tests: I think we need one structure for each
> of d3d, d3d2, d3d3, d3d7, where to have most of the things we store
> globally or in local vars now, and make the create/release functions
> work with that.
Yes, I was about to write something like that as a response to the cleanup 
patches.

PS: You forgot to CC wine-devel, I assume this was by accident so I've added 
it to the reply again. You may want to resend your original mail to wine-
devel.


signature.asc
Description: This is a digitally signed message part.