April 2015 - Wine-devel - list.winehq.org

Re: [PATCH 2/4] wined3d: Keep track of renderbuffer capabilities.
by Henri Verbeet 04 May '15

04 May '15

On 30 April 2015 at 00:00, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > diff --git a/dlls/wined3d/directx.c b/dlls/wined3d/directx.c > index 36be539..ad9b496 100644 > --- a/dlls/wined3d/directx.c > +++ b/dlls/wined3d/directx.c > @@ -4095,7 +4095,7 @@ static BOOL wined3d_check_pixel_format_color(const struct wined3d_gl_info *gl_in > BYTE redSize, greenSize, blueSize, alphaSize, colorBits; > > /* Float formats need FBOs. If FBOs are used this function isn't called */ > - if (format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_FLOAT) > + if (format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_FLOAT) That's not wrong as such, but it is pretty arbitrary. WINED3DFMT_FLAG_FLOAT is a basic property of the format, like e.g. the component masks and sizes, it shouldn't depend on the resource type. Once we'll get proper integer types for d3d10, it will probably make sense to keep track of the type (INT/UINT/FLOAT/UNORM/SNORM) as an enumeration instead, perhaps even per-component. > @@ -4178,8 +4178,8 @@ HRESULT CDECL wined3d_check_depth_stencil_match(const struct wined3d *wined3d, > ds_format = wined3d_get_format(&adapter->gl_info, depth_stencil_format_id); > if (wined3d_settings.offscreen_rendering_mode == ORM_FBO) > { > - if ((rt_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & WINED3DFMT_FLAG_RENDERTARGET) > - && (ds_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))) > + if ((rt_format->flags[WINED3D_GL_RES_TYPE_RB] & WINED3DFMT_FLAG_RENDERTARGET) > + && (ds_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))) This is actually wrong. At least, as long as we don't allow (plain) renderbuffers to be used for FBO attachments. There's nothing in the spec that says formats that can be used for color rendering with renderbuffers have to be available for color rendering with textures as well, and in fact that's much of the reason for this patch set. > @@ -998,9 +998,9 @@ static BOOL fbo_blit_supported(const struct wined3d_gl_info *gl_info, enum wined > break; > > case WINED3D_BLIT_OP_DEPTH_BLIT: > - if (!(src_format->flags[WINED3D_GL_RES_TYPE_TEX_2D] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))) > + if (!(src_format->flags[WINED3D_GL_RES_TYPE_RB] & (WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL))) Similar to WINED3DFMT_FLAG_FLOAT, these are basic properties of the format.

2 4

Re: [2/3] user32: Add some tests for EndDialog called from other thread.
by Marvin 30 Apr '15

30 Apr '15

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 https://testbot.winehq.org/JobDetails.pl?Key=13148 Your paranoid android. === w7u (32 bit msg) === msg.c:12230: Test failed: unexpected hwnd 0014013C msg.c:12252: Test failed: bad time 314e6/314f6

1 0

Re: [1/3] user32/tests: Add a test for calling SetWindowPos for other thread window. Resend.
by Marvin 30 Apr '15

30 Apr '15

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 https://testbot.winehq.org/JobDetails.pl?Key=13147 Your paranoid android. === w7u (32 bit msg) === msg.c:12230: Test failed: unexpected hwnd 0014013C msg.c:12252: Test failed: bad time 30ece/30ede

1 0

Re: [PATCH 2/5] d3d9/tests: Add a test for UpdateTexture. (v2, resend)
by Alexandre Julliard 30 Apr '15

30 Apr '15

Matteo Bruni <mbruni(a)codeweavers.com> writes: > v2: Make tests pass on the testbot, add a couple more test cases, some > small cleanups. It doesn't work here: ../../../tools/runtest -q -P wine -T ../../.. -M d3d9.dll -p d3d9_test.exe.so visual && touch visual.ok visual.c:12925: Tests skipped: No NVDB (depth bounds test) support, skipping tests. visual.c:16373: Tests skipped: DXT5 volume textures are not supported, skipping test. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 0, 1. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 0, 2. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 0, 3. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 0, 4. visual.c:18582: Test failed: Got unexpected color 0x0036c800, case 1, 1. visual.c:18582: Test failed: Got unexpected color 0x0036c800, case 1, 2. visual.c:18582: Test failed: Got unexpected color 0x0036c800, case 1, 3. visual.c:18582: Test failed: Got unexpected color 0x0036c800, case 1, 4. visual.c:18409: Tests skipped: Volume ATI2N textures are not supported, skipping some tests. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 2, 1. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 2, 2. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 2, 3. visual.c:18582: Test failed: Got unexpected color 0x0000ff00, case 2, 4. make: *** [visual.ok] Error 12 -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 4/4] wined3d: Don't use depth textures without ARB_depth_texture.
by Henri Verbeet 30 Apr '15

30 Apr '15

On 30 April 2015 at 00:00, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > + if (!gl_info->supported[ARB_DEPTH_TEXTURE]) > + { > + static const unsigned int depth_flags = WINED3DFMT_FLAG_SHADOW | WINED3DFMT_FLAG_DEPTH > + | WINED3DFMT_FLAG_STENCIL; > + format->flags[WINED3D_GL_RES_TYPE_TEX_1D] &= ~depth_flags; > + format->flags[WINED3D_GL_RES_TYPE_TEX_2D] &= ~depth_flags; > + format->flags[WINED3D_GL_RES_TYPE_TEX_3D] &= ~depth_flags; > + format->flags[WINED3D_GL_RES_TYPE_TEX_CUBE] &= ~depth_flags; > + format->flags[WINED3D_GL_RES_TYPE_TEX_RECT] &= ~depth_flags; > + } This is wrong for the reasons mentioned in patch 2/4, but it should also be unnecessary. WINED3DFMT_FLAG_TEXTURE is supposed to indicate if you can use a format for texturing or not.

1 0

Re: [PATCH 3/4] wined3d: Use renderbuffers if the resource type is RB.
by Henri Verbeet 30 Apr '15

30 Apr '15

On 30 April 2015 at 00:00, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > @@ -341,6 +341,8 @@ void wined3d_resource_update_draw_binding(struct wined3d_resource *resource) > resource->draw_binding = WINED3D_LOCATION_DRAWABLE; > else if (resource->multisample_type) > resource->draw_binding = WINED3D_LOCATION_RB_MULTISAMPLE; > + else if (resource->gl_type == WINED3D_GL_RES_TYPE_RB) > + resource->draw_binding = WINED3D_LOCATION_RB_RESOLVED; I haven't looked much into what actually happens now, but the last time I looked at using renderbuffers for FBO attachments, one of the issues that came up was the initial surface contents. Right now we handle that by clearing WINED3D_LOCATION_SYSMEM and marking it current, but that won't work so well for renderbuffers. Somewhat related, does WINED3DFMT_D16_LOCKABLE need special handling?

1 0

Re: [PATCH 1/4] wined3d: Allow specifying a different depth stencil location.
by Henri Verbeet 30 Apr '15

30 Apr '15

On 30 April 2015 at 00:00, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > @@ -2342,12 +2352,14 @@ BOOL context_apply_clear_state(struct wined3d_context *context, const struct win > ++i; > } > context_apply_fbo_state(context, GL_FRAMEBUFFER, context->blit_targets, > - wined3d_rendertarget_view_get_surface(fb->depth_stencil), > - rt_count ? rts[0]->resource->draw_binding : WINED3D_LOCATION_TEXTURE_RGB); > + wined3d_rendertarget_view_get_surface(dsv), > + rt_count ? rts[0]->resource->draw_binding : WINED3D_LOCATION_TEXTURE_RGB, > + dsv ? dsv->resource->draw_binding : WINED3D_LOCATION_TEXTURE_RGB); > } This works because we mostly just care that the color location isn't WINED3D_LOCATION_DRAWABLE if there's no attachment. (In particular for the case where the application is doing depth only rendering.) Strictly speaking though, it's probably more correct to use the color location if there's no depth/stencil attachment, to avoid silliness like the color attachment being in WINED3D_LOCATION_DRAWABLE and the depth/stencil attachment in WINED3D_LOCATION_TEXTURE_RGB. In fact, now that the color and depth/stencil locations are separate, you can probably just pass 0.

1 0

Re: [PATCH 5/5] wined3d: Don't use the same va_list multiple times in shader_vaddline().
by Henri Verbeet 30 Apr '15

30 Apr '15

On 29 April 2015 at 23:34, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > -static void string_buffer_vsprintf(struct wined3d_string_buffer *buffer, const char *format, va_list args) > +static int string_buffer_vsprintf(struct wined3d_string_buffer *buffer, const char *format, va_list args) > { > if (!buffer) > - return; > + return 0; > string_buffer_clear(buffer); > - shader_vaddline(buffer, format, args); > + return shader_vaddline(buffer, format, args); > } > You could perhaps just merge this into string_buffer_sprintf(), but I don't care that much.

1 0

Re: d3drm: Fix refcounting for IDirect3DRM{1-3}
by Stefan Dösinger 30 Apr '15

30 Apr '15

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-04-29 um 23:47 schrieb Aaryaman Vasishta: > - if (!refcount) > + if (!refcount && !InterlockedDecrement(&d3drm->numIfaces)) > HeapFree(GetProcessHeap(), 0, d3drm); This works, but this way you have the code that destroys the object 3 times. Not much of an issue as long as it is just a HeapFree, but I think it would be nicer to have a separate function. Either add a d3drm_destroy function or a pair of d3drm_add_iface and d3drm_release_iface functions that also take care of InterlockedIncrement / Decrement for iface_count) > - { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, TRUE }, > - { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, TRUE }, > + { &IID_IDirect3DRM3, &IID_IDirect3DRM3, S_OK, FALSE }, > + { &IID_IDirect3DRM2, &IID_IDirect3DRM2, S_OK, FALSE }, Afaics all the TODOs are gone, so you can remove this field from the table and the todo_wine code from test_qi(). Either in the same patch or a separate one. > - LONG ref; > + LONG ref1, ref2, ref3, numIfaces; > }; Please use iface_count. Yeah, it's just a plain style thing, but d3drm is nicely consistent so far in not using CamelCase (except in cases where Microsoft determined the name). (Yesterday on IRC I said num_ifaces. Henri says xxx_count is the way he prefers.) > + if (refcount == 1) InterlockedIncrement(&d3drm->numIfaces); Another minor style issue: Please put the statement on a separate line, in consistency with the existing code. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVQdzBAAoJEN0/YqbEcdMw5WAP/0H52sqgEXMdSmh6sXH/dZMQ nTZY9uRhElUbb3r6D6HG0ylit994pPaq887t9BSl8F4vbjsPXLjruDbVSwoScCfp rIDKy9xET+ek4p5TSFC9cX1BFAiC2VKN7YYWmr/wHKVPZc+aFudU9ykac02bi9p2 Oe+9wMBcFbwv33UhNL+2dugA4+0yj6egfPJWmju9C6ff6CPGqgXH70rHsrSf1AYs VkNPRhGpfI9BxojBChVtgc0zwPM1HnfOwfmKWCW75YNgNg9lL0o+bdoy2wCcxiLq FYwLLT2o9K+RnPQqRDhdJBKWwY8VrJI/fHZEURRau7RW1ia26j4UrrsAdXTQwhPs Iy8A63pYOxyZsphq5KJ8cwGABYIlBPUG0lXZQRgKh1Jw8f39HIR5cfnu1at/fPSc Cw6tfL/t+g4KeHaZDWZnROwyp704kIZGgvSaPuJjwkPtHh2MN14cp9KQj389kCUM 4KfV6FxsAByRuJ8ur1HSaV2OeIOKOJkaDTzaiiMqc581A0awOeCy5SuAUSrQu4IE 4gAIjryIJXd+kTDw46y6OW5UNXHdNJKGWAMiih25yLBi0BLBfUiW0U7qW5Lcerls e7zefeUYNT+Umj3WU4sDu6B8B6auSZgKrLu9JAwYH2RUhKX9R+z3uTY25VUbJkoP 9WJfJJuosIA5zqLbjxu+ =/BI/ -----END PGP SIGNATURE-----

2 1

Re: kernel32/tests: Add tests for deleting readonly files with NtCreateFile.
by Marvin 29 Apr '15

29 Apr '15

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 https://testbot.winehq.org/JobDetails.pl?Key=13137 Your paranoid android. === wvistau64 (32 bit file) === file.c:444: Test failed: attributes of file "testfile.xxx" are 0x0820 === w7u (32 bit file) === file.c:4052: Test failed: SetFileValidData failed 87 file.c:4055: Test failed: SetFileValidData failed 87 === wvistau64 (64 bit file) === file.c:444: Test failed: attributes of file "testfile.xxx" are 0x0820

1 0