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

Re: [PATCH 5/9] d3dx9: Store effect creation flags in struct d3dx_base_effect.
by Matteo Bruni 28 Apr '17

28 Apr '17

2017-04-27 13:05 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/effect.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/dlls/d3dx9_36/effect.c b/dlls/d3dx9_36/effect.c > index c2fb1d0..7885241 100644 > --- a/dlls/d3dx9_36/effect.c > +++ b/dlls/d3dx9_36/effect.c > @@ -154,6 +154,7 @@ struct d3dx9_base_effect > struct d3dx_object *objects; > > struct d3dx_effect_pool *pool; > + DWORD flags; > }; > > struct ID3DXEffectImpl > @@ -169,7 +170,7 @@ struct ID3DXEffectImpl > struct d3dx_technique *active_technique; > struct d3dx_pass *active_pass; > BOOL started; > - DWORD flags; > + DWORD begin_flags; begin_flags is quite ugly IMO. If you really want to change it from just flags, maybe call it creation_flags.

2 2

Re: [PATCH 4/9] d3dx9/tests: Add test for D3DXFX_LARGEADDRESSAWARE effect creation flag.
by Matteo Bruni 28 Apr '17

28 Apr '17

2017-04-27 13:05 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/d3dx9_36/tests/effect.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/dlls/d3dx9_36/tests/effect.c b/dlls/d3dx9_36/tests/effect.c > index 997c1fc..a58352e 100644 > --- a/dlls/d3dx9_36/tests/effect.c > +++ b/dlls/d3dx9_36/tests/effect.c > @@ -5743,6 +5743,30 @@ static void test_cross_effect_handle(IDirect3DDevice9 *device) > > effect2->lpVtbl->Release(effect2); > effect1->lpVtbl->Release(effect1); > + > + hr = D3DXCreateEffect(device, test_effect_preshader_effect_blob, sizeof(test_effect_preshader_effect_blob), > + NULL, NULL, D3DXFX_LARGEADDRESSAWARE, NULL, &effect1, NULL); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + param1 = effect1->lpVtbl->GetParameterByName(effect1, NULL, "g_iVect"); > + ok(!!param1, "GetParameterByName failed.\n"); > + > + hr = effect1->lpVtbl->SetValue(effect1, param1, expected_ivect, sizeof(expected_ivect)); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + hr = effect1->lpVtbl->GetValue(effect1, param1, ivect, sizeof(ivect)); > + ok(hr == D3D_OK, "Got result %#x.\n", hr); > + > + ok(!memcmp(ivect, expected_ivect, sizeof(expected_ivect)), "Vector value mismatch.\n"); > + > + if (0) > + { > + /* Native d3dx crashes in GetValue(). */ > + hr = effect1->lpVtbl->GetValue(effect1, "g_iVect", ivect, sizeof(ivect)); > + ok(hr == D3DERR_INVALIDCALL, "Got result %#x.\n", hr); > + } > + > + effect1->lpVtbl->Release(effect1); > } The test is okay but it doesn't feel right to put it in test_cross_effect_handle(). Probably just make a separate function.

1 0

Re: [PATCH 2/3] dwrite: Fix name section strings lookup
by Nikolay Sivov 27 Apr '17

27 Apr '17

On 27.04.2017 17:03, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/opentype.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > The problem with this patch is that we want to continue and add as many strings for different locales as possible. So the condition should set 'exists' to TRUE after first pair was added successfully and never touch it after.

2 3

Re: [PATCH 1/3] dwrite: Don't crash on uninitialized cached and factory fields in dwritefontface_Release
by Nikolay Sivov 27 Apr '17

27 Apr '17

On 27.04.2017 17:03, Piotr Caban wrote: > > The function may be called with the fields uninitialized from > create_fontface when init_font_data fails. > > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/font.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > > - factory_release_cached_fontface(This->cached); > - IDWriteFactory4_Release(This->factory); > + if (This->cached) > + factory_release_cached_fontface(This->cached); > + if (This->factory) > + IDWriteFactory4_Release(This->factory); Yes, this makes sense. The problem with this change is that 'cached' and 'factory' are not necessarily NULL on this failure path. I think at this point it would be better to use heap_alloc_zero(), and remove all explicit 0 initialization from create_fontface(). By the way, did you check why init_font_data() fails? If it's because opentype_get_font_familyname() fails, I'm interested in a log, or font file that causes it.

2 3

Re: [PATCH 3/3] dwrite: Fix EUDC font collection use after free issue
by Nikolay Sivov 27 Apr '17

27 Apr '17

On 27.04.2017 17:04, Piotr Caban wrote: > > Without this patch factory_detach_fontcollection incorrectly releases > factory reference. > > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/dwrite_private.h | 2 +- > dlls/dwrite/font.c | 6 ++++-- > dlls/dwrite/main.c | 15 ++++++++------- > 3 files changed, 13 insertions(+), 10 deletions(-) > This makes sense, thanks. Please add some test for that in test_GetEudc*, similar to what test_object_lifetime() does in the beginning - testing that collection gets factory reference, but only once.

1 0

Re: [PATCH 2/3] dwrite: Fix name section strings lookup
by Nikolay Sivov 27 Apr '17

27 Apr '17

On 27.04.2017 17:03, Piotr Caban wrote: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/dwrite/opentype.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > > - if (!(exists = opentype_decode_namerecord(header, storage_area, i, *strings))) > - continue; > + if ((exists = opentype_decode_namerecord(header, storage_area, i, *strings))) > + break; What happens exactly? I guess I'll need a font file for that too.

1 0

Re: [PATCH 1/13] msidb: Add stub tool for manipulating MSI databases.
by Hans Leidekker 27 Apr '17

27 Apr '17

On Sun, 2017-04-09 at 13:12 -0600, Erich E. Hoover wrote: > +static int open_database( struct msidb_state *state ) > +{ > + LPCWSTR db_mode = state->create_database ? MSIDBOPEN_CREATEDIRECT : MSIDBOPEN_DIRECT; > + UINT ret; > + > + ret = MsiOpenDatabaseW( state->database_file, db_mode, &state->database_handle ); Databases opened in direct mode should always be committed. You should either do that in following patches, or open them in transacted mode.

2 2

Re: [4/4] ole32: Fix IDataObject_GetDataHere may not implement
by Huw Davies 27 Apr '17

27 Apr '17

On Wed, Apr 26, 2017 at 03:45:50PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 2c5a9b4df394c6d50bcc24bcc7b489a864224155 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:24:00 +0800 > Subject: [PATCH 4/4] ole32: Fix IDataObject_GetDataHere may not implement > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index a0c4445..5ee031e 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -115,6 +115,10 @@ static HRESULT get_storage(IDataObject *data, IStorage *stg, UINT *src_cf, BOOL > med.u.pstg = stg; > med.pUnkForRelease = NULL; > hr = IDataObject_GetDataHere(data, &fmt, &med); > + if (FAILED(hr)) > + { > + hr = IDataObject_GetData(data, &fmt, &med); > + } This can't work. The STGMEDIUM is an [out]-only parameter of _GetData(), meaning that the callee will return a new stg in the STGMEDIUM. If this is indeed called (we need tests to check), then you'd have to copy the returned stg into the original one. Huw.

1 0

Re: [3/4] ole32: Add init cache code after OleLoad in OleCreateFromDataEx
by Huw Davies 27 Apr '17

27 Apr '17

On Wed, Apr 26, 2017 at 03:46:23PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From a96e73f270745f0694ea340eb8119566d75f1117 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:14:36 +0800 > Subject: [PATCH 3/4] ole32: Add init cache code after OleLoad in > OleCreateFromDataEx > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index ccf31fa..a0c4445 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -181,6 +181,7 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > { > HRESULT hr; > UINT src_cf; > + IOleCache* ole_cache; > > FIXME("(%p, %s, %08x, %08x, %d, %p, %p, %p, %p, %p, %p, %p): stub\n", > data, debugstr_guid(iid), flags, renderopt, num_cache_fmts, adv_flags, cache_fmts, > @@ -193,6 +194,11 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > if(FAILED(hr)) return hr; > > /* FIXME: Init cache */ > + hr = IUnknown_QueryInterface((IUnknown*)*obj, &IID_IOleCache, (void**)&ole_cache); > + if (SUCCEEDED(hr)) > + { > + hr = IOleCache_InitCache(ole_cache, data); > + } You'll need to release the ole_cache. Again we'll need some tests. Huw.

1 0

Re: [2/4] ole32: Support HBITMAP format in DataCache_GetExtent and DataCache_Draw
by Huw Davies 27 Apr '17

27 Apr '17

On Wed, Apr 26, 2017 at 03:44:01PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 91b46bc9da357b76b8028d058a191743e9a31a32 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:05:59 +0800 > Subject: [PATCH 2/4] ole32: Support HBITMAP format in DataCache_GetExtent and > DataCache_Draw > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/datacache.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > @@ -1908,6 +1926,40 @@ static HRESULT WINAPI DataCache_GetExtent( > > return S_OK; > } > + case CF_BITMAP: > + { > + HDC hdc; > + char dst_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )]; > + BITMAPINFO *dst_info = (BITMAPINFO *)dst_bmibuf; > + BITMAPINFOHEADER *info; > + LONG x_pels_m, y_pels_m; > + > + if (cache_entry->stgmedium.tymed != TYMED_GDI) > + continue; > + > + hdc = GetDC(NULL); > + memset(dst_info, 0, sizeof(char)*FIELD_OFFSET( BITMAPINFO, bmiColors[256] )); > + dst_info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); > + > + GetDIBits(hdc, cache_entry->stgmedium.u.hBitmap, 0, 1, NULL, dst_info, DIB_RGB_COLORS); > + > + info = &dst_info->bmiHeader; I'm not convinced adding the info variable helps here. Just use dst_info->bmiHeader throughout. Better still drop the 'dst_' and use BITMAPINFO *info = (BITMAPINFO *)bmibuf; > + x_pels_m = info->biXPelsPerMeter; > + y_pels_m = info->biYPelsPerMeter; > + > + /* Size in units of 0.01mm (ie. MM_HIMETRIC) */ > + if (x_pels_m != 0 && y_pels_m != 0) > + { > + lpsizel->cx = info->biWidth * 100000 / x_pels_m; > + lpsizel->cy = info->biHeight * 100000 / y_pels_m; > + } > + else > + { > + lpsizel->cx = info->biWidth * 2540 / GetDeviceCaps( hdc, LOGPIXELSX ); > + lpsizel->cy = info->biHeight * 2540 / GetDeviceCaps( hdc, LOGPIXELSY ); > + } You need a ReleaseDC(0, hdc); > + return S_OK; > + } > } > } > > -- > 2.10.2 > >

1 0