January 2016 - Wine-Devel - list.winehq.org

Re: comctl32: treeview: Fix crash when deleting the first visible item while bRedraw == false

by Nikolay Sivov

On 01/30/2016 12:11 AM, Joachim Priesner wrote: > When bRedraw == false, TREEVIEW_DeleteItem returned early, leaving > firstVisible == NULL. This led to a crash when bRedraw was set to true > again. > > Signed-off-by: Joachim Priesner <joachim.priesner(a)web.de> > --- > dlls/comctl32/tests/treeview.c | 38 ++++++++++++++++++++++++++++++++++++++ > dlls/comctl32/treeview.c | 4 +++- > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/dlls/comctl32/tests/treeview.c b/dlls/comctl32/tests/treeview.c > index 92504c1..34d0674 100644 > --- a/dlls/comctl32/tests/treeview.c > +++ b/dlls/comctl32/tests/treeview.c > @@ -1903,6 +1903,43 @@ static void test_delete_items(void) > DestroyWindow(hTree); > } > > +static void test_delete_first_visible_setredraw_false(void) Please append new tests to test_delete_items(), after a comment line describing what's being tested. > +{ > + HWND hTree; > + INT ret; > + TVINSERTSTRUCTA ins; > + HTREEITEM hItem1; > + > + static CHAR item1[] = "Item 1"; > + static CHAR item2[] = "Item 2"; > + > + hTree = create_treeview_control(0); > + > + ins.hParent = TVI_ROOT; > + ins.hInsertAfter = TVI_ROOT; > + U(ins).item.mask = TVIF_TEXT; > + U(ins).item.pszText = item1; > + hItem1 = TreeView_InsertItemA(hTree, &ins); Just to make sure add a test for hItem != NULL. > + > + ins.hParent = TVI_ROOT; > + ins.hInsertAfter = hItem1; > + U(ins).item.mask = TVIF_TEXT; > + U(ins).item.pszText = item2; > + (void*) TreeView_InsertItemA(hTree, &ins); /* return value unused */ > + Same here, it'll also make a value used. > + ret = SendMessageA(hTree, WM_SETREDRAW, 0, 0); > + ok(ret == 0, "got %d\n", ret); > + > + ret = SendMessageA(hTree, TVM_DELETEITEM, 0, (LPARAM)hItem1); > + ok(ret == TRUE, "got %d\n", ret); > + > + /* this used to crash, test that it does not any more */ > + ret = SendMessageA(hTree, WM_SETREDRAW, 1, 0); > + ok(ret == 0, "got %d\n", ret); > + For WM_SETREDRAW wParam please use TRUE/FALSE instead. Otherwise looks good, thanks for fixing this.

Jan. 31, 2016

Re: [v2 PATCH 1/3] gdiplus: Implement GdipGetPenTransform

by Nikolay Sivov

On Sunday, 31 January 2016, Alistair Leslie-Hughes < leslie_alistair(a)hotmail.com> wrote: > Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com > <javascript:;>> > --- > dlls/gdiplus/gdiplus_private.h | 1 + > dlls/gdiplus/pen.c | 9 ++++----- > dlls/gdiplus/tests/pen.c | 6 +++--- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/dlls/gdiplus/gdiplus_private.h > b/dlls/gdiplus/gdiplus_private.h > index 1af0bd0..fd45a49 100644 > --- a/dlls/gdiplus/gdiplus_private.h > +++ b/dlls/gdiplus/gdiplus_private.h > @@ -197,6 +197,7 @@ struct GpPen{ > REAL offset; /* dash offset */ > GpBrush *brush; > GpPenAlignment align; > + GpMatrix *transform; > }; Why does it have to be a separate allocation? It looks like GpMatrix field will work fine.

Jan. 31, 2016

Re: [PATCH 2/2] mscoree: call native DllEntryPoint when it is defined in COR header.

by Vincent Povirk

You seem to have some weird indentation? We should probably still call DisableThreadLibraryCalls on process attach if there's no native entry point.

Jan. 29, 2016

Re: jscript: Fixed tests on some old jscript versions.

by 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 https://testbot.winehq.org/JobDetails.pl?Key=20096 Your paranoid android. === wxppro (32 bit) === No test summary line found === w2003std (32 bit) === No test summary line found === wvistau64 (32 bit) === No test summary line found === w2008s64 (32 bit) === No test summary line found === w7u (32 bit) === No test summary line found === w7pro64 (32 bit) === No test summary line found === w8 (32 bit) === No test summary line found === w864 (32 bit) === No test summary line found === w1064 (32 bit) === No test summary line found === wvistau64 (64 bit) === No test summary line found === w2008s64 (64 bit) === No test summary line found === w7pro64 (64 bit) === No test summary line found === w864 (64 bit) === No test summary line found === w1064 (64 bit) === No test summary line found

Jan. 29, 2016

Re: [PATCH 3/6] d3d11/tests: Add a test for CheckMultisampleQualityLevels.

by Henri Verbeet

On 29 January 2016 at 00:51, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > +static void test_checkmultisamplequalitylevels(void) The naming here is a bit different from the other tests. :) I guess it was inspired by the equivalent d3d9 test?

Jan. 29, 2016

Re: [PATCH 6/6] d3d10core/tests: Test the creation of textures with invalid multisample settings.

by Henri Verbeet

On 29 January 2016 at 11:15, Józef Kucia <joseph.kucia(a)gmail.com> wrote: > This is fine by me but it could be considered to be merged into > test_create_texture2d(). > I do think that would be a good fit for this, although I don't feel particularly strongly about it either.

Jan. 29, 2016

Re: [PATCH 3/5] wined3d: Recognize SM4 dcl_immediateConstantBuffer.

by Henri Verbeet

On 29 January 2016 at 09:44, Józef Kucia <jkucia(a)codeweavers.com> wrote: > + else if (ins.handler_idx == WINED3DSIH_DCL_IMMEDIATE_CONSTANT_BUFFER) > + { > + struct wined3d_shader_immediate_constant_buffer *icb = ins.declaration.icb; > + if (reg_maps->icb) > + { > + ERR("Multiple immediate constant buffers.\n"); This would need to be a WARN or a FIXME, unless you ensure that it can never happen in the front-end. > + HeapFree(GetProcessHeap(), 0, reg_maps->icb); > + } > + if (!(reg_maps->icb = HeapAlloc(GetProcessHeap(), 0, icb->element_count * sizeof(*icb->data)))) > + { > + ERR("Failed to allocate immediate constant buffer.\n"); > + return E_OUTOFMEMORY; > + } > + memcpy(reg_maps->icb, icb->data, icb->element_count * sizeof(*icb->data)); > + reg_maps->icb_size = icb->element_count / 4; I think it shouldn't be necessary to copy the data, shader->frontend_data should stay alive until the shader is destroyed. > + FIXME("Unexpected size of immediate constant buffer %u.\n", len); "Unexpected immediate constant buffer size %u.\n" sounds more natural to me, but it's not a big deal.

Jan. 29, 2016

Re: [PATCH 1/6] d3d11: Implement d3d11_device_CheckMultisampleQualityLevels().

by Henri Verbeet

On 29 January 2016 at 00:51, Matteo Bruni <mbruni(a)codeweavers.com> wrote: > + if (sample_count > 32) > + return E_FAIL; I think that's D3D11_MAX_MULTISAMPLE_SAMPLE_COUNT. > + if (sample_count > 16) > + { > + FIXME("sample_count %u not handled yet.\n", sample_count); > + return S_OK; > + } I think that belongs in wined3d_check_device_multisample_type(). > + hr = wined3d_device_check_multisample_quality_levels(device->wined3d_device, > + wined3dformat_from_dxgi_format(format), sample_count, quality_level_count); > + if (hr == WINED3DERR_INVALIDCALL) > + return E_INVALIDARG; > + if (hr == WINED3DERR_NOTAVAILABLE) > + return S_OK; This is fine, but the d3d10+ API probably makes more sense in the long run. I.e., eventually we'd want to handle the difference in d3d8/9 instead. > +HRESULT CDECL wined3d_device_check_multisample_quality_levels(const struct wined3d_device *device, > + enum wined3d_format_id format_id, enum wined3d_multisample_type multisample_type, > + DWORD *quality_level_count) I don't think we really want to introduce a new entry point for this. I guess the issue is that we can't get at the wined3d object that created the device from inside d3d11, but I think it would be preferable to do something about that instead. It seems likely that CheckFeatureSupport() and CheckFormatSupport() would run into this as well.

Jan. 29, 2016

Re: [PATCH v3] ntoskrnl.exe: Implement IoGetDeviceProperty for DevicePropertyPhysicalDeviceObjectName

by Aric Stewart

On 1/29/16 8:29 AM, Alexandre Julliard wrote: > Aric Stewart <aric(a)codeweavers.com> writes: > >> + case DevicePropertyPhysicalDeviceObjectName: >> + { >> + ULONG used_len; >> + OBJECT_NAME_INFORMATION *name = HeapAlloc(GetProcessHeap(), 0, buffer_length + (sizeof(OBJECT_NAME_INFORMATION))); >> + >> + status = NtQueryObject(device->Reserved, ObjectNameInformation, name, buffer_length + sizeof(OBJECT_NAME_INFORMATION), &used_len); >> + if (status == STATUS_SUCCESS) >> + { >> + memcpy(property_buffer, name->Name.Buffer, name->Name.Length); >> + *result_length = name->Name.Length; >> + } >> + else >> + *result_length = 0; > > Length is still not correct if the buffer is too small, and the string > needs to be null-terminated. > Sorry you are correct I am trying to focus on too many divergent things at once and my brain has stated to turn to mush. -aric

Jan. 29, 2016

Re: [PATCH v3] ntoskrnl.exe: Implement IoGetDeviceProperty for DevicePropertyPhysicalDeviceObjectName

by Alexandre Julliard

Aric Stewart <aric(a)codeweavers.com> writes: > + case DevicePropertyPhysicalDeviceObjectName: > + { > + ULONG used_len; > + OBJECT_NAME_INFORMATION *name = HeapAlloc(GetProcessHeap(), 0, buffer_length + (sizeof(OBJECT_NAME_INFORMATION))); > + > + status = NtQueryObject(device->Reserved, ObjectNameInformation, name, buffer_length + sizeof(OBJECT_NAME_INFORMATION), &used_len); > + if (status == STATUS_SUCCESS) > + { > + memcpy(property_buffer, name->Name.Buffer, name->Name.Length); > + *result_length = name->Name.Length; > + } > + else > + *result_length = 0; Length is still not correct if the buffer is too small, and the string needs to be null-terminated. -- Alexandre Julliard julliard(a)winehq.org

Jan. 29, 2016