On 26 November 2014 at 19:08, Stefan Dösinger stefan@codeweavers.com wrote:
conv.f = tests[i].start;hr = IDirect3DDevice8_SetRenderState(device, D3DRS_FOGSTART, conv.d);
I don't care particularly much, but it might be nicer to just add the union to tests[], and then just use "tests[i].start.d" here.
START_TEST(ddraw7) { HMODULE module = GetModuleHandleA("ddraw.dll"); @@ -8089,4 +8190,5 @@ START_TEST(ddraw7) test_resource_priority(); test_surface_desc_lock(); fog_interpolation_test();
- test_negative_fixedfunction_fog();
I prefer this name, but I think it's more important that equivalent tests have the same name between D3D versions. I'd probably just rename the d3d8 and d3d9 versions instead. For what it's worth, I still plan to merge device.c and visual.c at some point in the future, and I'll probably fixup the naming of the existing tests in visual.c around that time as well.
if (settings->ortho_fog)
{ /* Need to undo the [0.0 - 1.0] -> [-1.0 - 1.0] transformation from D3D to GL coordinates. */ shader_addline(buffer, "gl_FogFragCoord = gl_Position.z * 0.5 + 0.5;\n");} else{ shader_addline(buffer, "gl_FogFragCoord = ec_pos.z;\n");if (!settings->transformed)shader_addline(buffer, "gl_FogFragCoord = abs(gl_FogFragCoord);\n");}
I think this looks pretty messy. I'd suggest
if (settings->ortho_fog) shader_addline(...); else if (!settings->transformed) shader_addline(...); else shader_addline(...);
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2014-11-27 10:22, schrieb Henri Verbeet:
For what it's worth, I still plan to merge device.c and visual.c at some point in the future,
I don't like this idea. I prefer to be able to run the visual tests on Windows without the mode setting tests in device.c messing up my screen and taking forever on my slow monitor. I'm also not a fan of 30k lines of code in a file, although that doesn't mean we have to go to the opposite extreme and add hundreds of 50-line .java files.
(Yeah, I can comment out unused tests while working on a new one, but that's awkward too.)
On 27 November 2014 at 10:47, Stefan Dösinger stefandoesinger@gmail.com wrote:
For what it's worth, I still plan to merge device.c and visual.c at some point in the future,
I don't like this idea. I prefer to be able to run the visual tests on Windows without the mode setting tests in device.c messing up my screen and taking forever on my slow monitor. I'm also not a fan of 30k lines of code in a file, although that doesn't mean we have to go to the opposite extreme and add hundreds of 50-line .java files.
Actually, new tests should in general go into device.c although we haven't really been enforcing that, in part because we currently don't have the get_surface_color() / getPixelColor() helpers in device.c. Merging would help with duplication of helper functions, and also be more consistent with the ddraw, d3d10core and d2d1 tests. D3d9 in particular is weird because it has the device/visual split for d3d9, but a single file for d3d9ex.
Perhaps some of the mode setting tests can be merged together, although obviously at the cost of increased complexity, or perhaps we can e.g. introduce a WINETEST_ environment variable to avoid running them. I think improving the mode setting tests is a separate issue from merging device.c and visual.c though.
As an aside, many of the Present() calls in visual.c are really only useful when debugging, and just removing them may speed things up as well, although I haven't verified that.
(Yeah, I can comment out unused tests while working on a new one, but that's awkward too.)
I pretty much already always do that. In part because it helps to avoid depending on side effects of previous tests, and in part because it's just so much faster, especially on the testbot.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Am 2014-11-27 11:31, schrieb Henri Verbeet:
Actually, new tests should in general go into device.c although we haven't really been enforcing that, in part because we currently don't have the get_surface_color() / getPixelColor() helpers in device.c. Merging would help with duplication of helper functions, and also be more consistent with the ddraw, d3d10core and d2d1 tests. D3d9 in particular is weird because it has the device/visual split for d3d9, but a single file for d3d9ex.
Well, there's one point of separation: device.c and d3d9ex.c test device and resource creation, which is an area with differences between d3d9 and d3d9ex. Visual.c tests rendering once resources are created, and so far we haven't seen a difference here between d3d9 and d3d9ex.
Is there some overlap between device.c and visual.c? Sure. But I don't think that means we should put everything into one huge file.
Ddraw is somewhat different because the differences between ddraw versions are bigger and actually affect rendering.
(Note that this isn't something I'd wager a holy war about. I just think fairly minor overlap between device.c and visual.c leads you to the opposite extreme of Java's tiny files.)
Perhaps some of the mode setting tests can be merged together, although obviously at the cost of increased complexity,
I'm pessimistic about that. I looked a bit into it (not much I must admit, just a bit), and I ran into problems pretty quickly, e.g. that the w8 ddraw driver crashes when calling GetSurfaceDesc after an external mode change. Reactivating a device on Windows in d3d8/9 is tricky, fvwm doesn't make this any easier on the other side, etc. I don't think we'll be able to cut down the number of mode changes substantially without sacrificing reliability.
-
Henri Verbeet -
Stefan Dösinger