Mostly just a couple of style comments; feel free to disregard if you like:
The subject line, comments, and ok messages are missing full stops.
Personally, I'm not a fan of the switch/case style of tests. My preference would be to either just explicitly write out the tests, or to encode them using the "tests[]" construction we use in a couple of other tests.
+ static const D3D11_VIEWPORT viewport = {0, 0, 2, 1, 0, 1}; ... + texture_desc.Width = (UINT)viewport.Width; + texture_desc.Height = (UINT)viewport.Height; ... + ID3D11DeviceContext_RSSetViewports(context, 1, &viewport);
That works, but more typically we'd initialise "texture_desc", and then call "set_viewport(..., texture_desc.Width, texture_desc.Height, ...)".
+ int test;
"test" can't be negative, right?
+ unsigned int i; + static const UINT vertex_stride = sizeof(depth_quarter_quad_data[0]); + static const UINT vertex_offset = 0; + static const struct vec4 initial_color = {0, 0, 0, 1}; + static const float clear_color[] = {0, 1, 0, 1}; + DWORD expected_color[] = {0xff000000, 0xff000000}; + FLOAT expected_depth[] = {0.5f, 0.5f};
The choice for "FLOAT" seems odd, given that get_readback_float() returns a float. We don't tend to use UINT and DWORD a lot either.
+ for (i = 0; i < ARRAYSIZE(expected_color); i++)
It's not wrong, but all the existing code uses ARRAY_SIZE...
+ (!damavand && test == 3 && i == 0) /* Draw with no PS doesn't write color on D3D11 but does on OpenGL */
I don't think the comment is terribly clear, but it sounds like we're getting fixed-function with the GL backend, and things just happen to work with the Vulkan backend because we don't yet have a fixed-function implementation there. That likely won't last forever though. For comparison, vkd3d handles this explicitly in d3d12_pipeline_state_init_graphics().
+ ok(depth == expected_depth[i], "Test %d: Got unexpected depth %.2f at %u\n", test, depth, i);
I don't think %.2f is quite sufficient to show what's going on if the result is only off by a couple of ulps. That's perhaps not terribly likely for clears with carefully chosen depth values, but it still looks a little sketchy to me.
+static void wined3d_context_prepare_used_framebuffer(struct wined3d_rendertarget_view *rtv, struct wined3d_context *context) +{ + if (wined3d_rendertarget_view_get_locations(rtv) & WINED3D_LOCATION_CLEARED) + { + /* Need to rebind framebuffer or the clear won't happen */ + context_invalidate_state(context, STATE_FRAMEBUFFER); + wined3d_rendertarget_view_prepare_location(rtv, context, rtv->resource->draw_binding); + } + else + { + wined3d_rendertarget_view_load_location(rtv, context, rtv->resource->draw_binding); + } +}
The comment doesn't seem quite as helpful as it perhaps could be; if I understand the issue correctly, we don't actually care about rebinding the framebuffer, but instead about starting a new render pass, because that's where we'll clear these. That's also what the commit message seems to suggest.
By convention, "context" would be the first parameter for wined3d_context_*() functions. The function name is also slightly odd, because it's preparing/loading individual views, not complete framebuffers.
-
Henri Verbeet (＠hverbeet)