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.
On Tue, Nov 8, 2011 at 12:18 PM, Stefan Dösinger stefandoesinger@gmx.at 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
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(999999). 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.
-
Octavian Voicu -
Stefan Dösinger