October 2012 - Wine-Devel - list.winehq.org

Re: [PATCH 4/5] d3dx9_36: Implement ID3DXFileEnumObjectImpl_GetChildren & ID3DXFileEnumObjectImpl_GetChild and add stubbed interface for ID3DXFileData. (try 4)

by Nikolay Sivov

On 10/25/2012 10:34, Christian Costa wrote: > > + > +static HRESULT WINAPI ID3DXFileDataImpl_Create(IDirectXFileObject *dxfile_object, ID3DXFileData **ret_iface) > +{ > + ID3DXFileDataImpl *object; > + > + TRACE("(%p, %p)\n", dxfile_object, ret_iface); > + > + *ret_iface = NULL; > + > + object = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*object)); > + if (!object) > + { > + ERR("Out of memory\n"); > + return E_OUTOFMEMORY; > + } > + > + object->ID3DXFileData_iface.lpVtbl = &ID3DXFileData_Vtbl; > + object->ref = 1; > + object->dxfile_object = (IDirectXFileData*)dxfile_object; > + > + *ret_iface = &object->ID3DXFileData_iface; > + > + return S_OK; > +} > + You don't need WINAPI for it. > > + ret = ID3DXFileDataImpl_Create((IDirectXFileObject*)data_object, &object->children[object->nb_children]); It seems you're casting it back and forth, is it possible to use proper type for data_object? > + { > + (&object->ID3DXFileEnumObject_iface)->lpVtbl->Release(&object->ID3DXFileEnumObject_iface); Hmm, what's wrong with object->ID3DXFileEnumObject_iface.lpVtbl->Release ?

Oct. 25, 2012

Re: [PATCH 5/5] d3dx9_36: Implement ID3DXFileDataImpl_GetType. (try 5)

by Christian Costa

2012/10/25 Nikolay Sivov <bunglehead(a)gmail.com> > On 10/25/2012 10:34, Christian Costa wrote: > >> --- >> dlls/d3dx9_36/xfile.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/dlls/d3dx9_36/xfile.c b/dlls/d3dx9_36/xfile.c >> index a1e0596..4c216da 100644 >> --- a/dlls/d3dx9_36/xfile.c >> +++ b/dlls/d3dx9_36/xfile.c >> @@ -154,9 +154,19 @@ static HRESULT WINAPI ID3DXFileDataImpl_Unlock(**ID3DXFileData >> *iface) >> static HRESULT WINAPI ID3DXFileDataImpl_GetType(**ID3DXFileData >> *iface, GUID *guid) >> { >> - FIXME("(%p)->(%p): stub\n", iface, guid); >> + ID3DXFileDataImpl *This = impl_from_ID3DXFileData(iface)**; >> + const GUID *dxfile_guid; >> + HRESULT ret; >> - return E_NOTIMPL; >> + TRACE("(%p)->(%p)\n", iface, guid); >> + >> + ret = IDirectXFileData_GetType(This-**>dxfile_object, &dxfile_guid); >> + if (ret != S_OK) >> + return ret; >> + >> + *guid = *dxfile_guid; >> + >> + return S_OK; >> } >> >> > Why not simply forward to IDirectXFileData here? What is all the > surrounding code for? > > > > IDirectXFileData_GetType returns a pointer to a GUID (GUID**) whereas ID3DXFileData_GetType fill the GUID (GUID*) hence this code.

Oct. 25, 2012

Add tests fo D3DXSHRotateZ

by Nozomi Kodama

Hello, Is this patch not good enough to be commited? Nozomi

Oct. 25, 2012

Re: [PATCH 1/5] d3dx9_36: Implement D3DXFileCreate. (try 3)

by Dmitry Timoshkov

Rico Schüller <kgbricola(a)web.de> wrote: > Well the app may initialize some resources (some of them fail with out > of memory) and run half an hour. If it accesses the invalid pointer it > crashes. Do you see the problem from the backtrace? The real problem > happened half an hour ago and the err message doesn't hurt anyone, it > just shows the problem as early as possible, which I think is better > than to relay on a eventually crashing app at some point. The err > doesn't spam the console, since it only happens when there is a problem. If you allocate a large chunk of memory it's probably justified to print a WARN(), but when allocating a structure of a couple tens of bytes it's already too late. Inside a library (and Wine is a library) printing anything to the console should be avoided as much as possible with an obvious exception for not implemented things. -- Dmitry.

Oct. 25, 2012

Re: [PATCH(0/1)] wininet: TLS fallback mechanism

by Hiroshi Miura

Hiroshi Miura wrote: > Evernote windows client on Wine/OpenSSL 1.0.1/Ubuntu12.04/12.10 cannot connect > its server. > It is caused by server is only support TLS1.0/SSL3.0, > but client ask TLS1.1/1.2 and then server returns that session is fails. > > A wininet in Microsoft Windows7 implementation has a behavior > to re-try by TLS1.0 after TLS1.2 fails and got FIN from server. > > This patch enabled fall back mechanism to SSLv3/TLSv1. > Here is details of behavior when patched. Attached is a captured by 'ssldump -AH -i <interface>' when running https://gist.github.com/3949057 test program that makes binary by winemaker. When original, connection is failed here. > 1 0.2624 (0.1304) S>C TCP FIN With fall back mechanism, wininet.dll try to connect again with > ClientHello > Version 3.1 then success to connect. ----------------------------------- New TCP connection #1: miurahr-note.local(54342) <-> www.evernote.com(443) 1 1 0.1319 (0.1319) C>SV3.1(221) Handshake ClientHello Version 3.2 random[32]= 50 87 fb 25 93 f9 4d c6 f2 14 87 70 0a 9f 5b 37 b0 e2 ef 43 76 8c de 31 b9 a8 47 7e 74 6c 15 85 cipher suites Unknown value 0xc014 Unknown value 0xc00a Unknown value 0xc022 Unknown value 0xc021 TLS_DHE_RSA_WITH_AES_256_CBC_SHA TLS_DHE_DSS_WITH_AES_256_CBC_SHA Unknown value 0x88 Unknown value 0x87 Unknown value 0xc00f Unknown value 0xc005 TLS_RSA_WITH_AES_256_CBC_SHA Unknown value 0x84 Unknown value 0xc012 Unknown value 0xc008 Unknown value 0xc01c Unknown value 0xc01b TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA Unknown value 0xc00d Unknown value 0xc003 TLS_RSA_WITH_3DES_EDE_CBC_SHA Unknown value 0xc013 Unknown value 0xc009 Unknown value 0xc01f Unknown value 0xc01e TLS_DHE_RSA_WITH_AES_128_CBC_SHA TLS_DHE_DSS_WITH_AES_128_CBC_SHA Unknown value 0x9a Unknown value 0x99 Unknown value 0x45 Unknown value 0x44 Unknown value 0xc00e Unknown value 0xc004 TLS_RSA_WITH_AES_128_CBC_SHA Unknown value 0x96 Unknown value 0x41 Unknown value 0xc011 Unknown value 0xc007 Unknown value 0xc00c Unknown value 0xc002 TLS_RSA_WITH_RC4_128_SHA TLS_RSA_WITH_RC4_128_MD5 TLS_DHE_RSA_WITH_DES_CBC_SHA TLS_DHE_DSS_WITH_DES_CBC_SHA TLS_RSA_WITH_DES_CBC_SHA TLS_DHE_RSA_EXPORT_WITH_DES40_CBC_SHA TLS_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA TLS_RSA_EXPORT_WITH_DES40_CBC_SHA TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5 TLS_RSA_EXPORT_WITH_RC4_40_MD5 Unknown value 0xff compression methods unknown value NULL 1 0.2624 (0.1304) S>C TCP FIN 1 0.2625 (0.0000) C>S TCP FIN New TCP connection #2: miurahr-note.local(54343) <-> www.evernote.com(443) 2 1 0.1268 (0.1268) C>SV3.1(221) Handshake ClientHello Version 3.1 random[32]= 50 87 fb 25 65 48 ce a6 93 20 b4 d6 f6 d9 49 0d 7b db 7a 93 3c 89 32 4d 4d 15 bc f2 dd ef 26 79 cipher suites Unknown value 0xc014 Unknown value 0xc00a Unknown value 0xc022 Unknown value 0xc021 TLS_DHE_RSA_WITH_AES_256_CBC_SHA TLS_DHE_DSS_WITH_AES_256_CBC_SHA Unknown value 0x88 Unknown value 0x87 Unknown value 0xc00f Unknown value 0xc005 TLS_RSA_WITH_AES_256_CBC_SHA Unknown value 0x84 Unknown value 0xc012 Unknown value 0xc008 Unknown value 0xc01c Unknown value 0xc01b TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA Unknown value 0xc00d Unknown value 0xc003 TLS_RSA_WITH_3DES_EDE_CBC_SHA Unknown value 0xc013 Unknown value 0xc009 Unknown value 0xc01f Unknown value 0xc01e TLS_DHE_RSA_WITH_AES_128_CBC_SHA TLS_DHE_DSS_WITH_AES_128_CBC_SHA Unknown value 0x9a Unknown value 0x99 Unknown value 0x45 Unknown value 0x44 Unknown value 0xc00e Unknown value 0xc004 TLS_RSA_WITH_AES_128_CBC_SHA Unknown value 0x96 Unknown value 0x41 Unknown value 0xc011 Unknown value 0xc007 Unknown value 0xc00c Unknown value 0xc002 TLS_RSA_WITH_RC4_128_SHA TLS_RSA_WITH_RC4_128_MD5 TLS_DHE_RSA_WITH_DES_CBC_SHA TLS_DHE_DSS_WITH_DES_CBC_SHA TLS_RSA_WITH_DES_CBC_SHA TLS_DHE_RSA_EXPORT_WITH_DES40_CBC_SHA TLS_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA TLS_RSA_EXPORT_WITH_DES40_CBC_SHA TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5 TLS_RSA_EXPORT_WITH_RC4_40_MD5 Unknown value 0xff compression methods unknown value NULL 2 2 0.8834 (0.7566) S>CV3.1(74) Handshake ServerHello Version 3.1 random[32]= 50 87 fb 29 9f 05 e2 82 c0 97 fd a3 d7 0d bd 67 d2 a6 42 47 24 e5 d1 c0 d2 e3 42 d5 23 23 00 b0 session_id[32]= 50 87 fb 29 9f 05 e2 82 c0 97 fd a3 d7 0d bd 67 d2 a6 42 47 24 e5 d1 c0 d2 e3 42 d5 23 23 00 b0 cipherSuite TLS_RSA_WITH_AES_256_CBC_SHA compressionMethod NULL 2 3 1.0231 (0.1396) S>CV3.1(2953) Handshake Certificate certificate[1356]= 30 82 05 48 30 82 04 30 a0 03 02 01 02 02 10 5d a1 43 88 66 ca 05 04 e1 4f 00 b4 71 30 67 fe 30 <SNIP> cc a2 9a f1 6e e8 cf 8e d1 1a 3c 5e 19 c5 d7 9b 35 b0 02 23 24 e5 05 b8 d5 88 e3 e0 fa b9 f4 5f 2 4 1.0231 (0.0000) S>CV3.1(4) Handshake ServerHelloDone

Oct. 24, 2012

Re: windowscodecs questions.

by Max TenEyck Woodbury

On 10/24/2012 02:18 PM, Vincent Povirk wrote: >> I found some code that reads a generic chunk. I see where the size >> and type are read, where a correctly sized record is allocated and >> where the data is read into the record. > > That code is unused for reading and writing PNG images right now, > sorry for the confusion. Libpng has its own code for reading and > writing PNG chunks, and that is what is being used. Our chunk code is > used for PNG metadata handlers (which will have to work independently > from libpng so we can have a proper, pluggable system for metadata), > but the architecture isn't there yet to use those handlers when > reading a PNG image. > >> I added code to read and check the CRC to my working copy of the git >> tree. That threw off the block synchronization, so I think there is >> code someplace else that either checks or skips the CRC, but I have >> not found it after a few hours searching for it. An indication on >> where to look and other advice would be very helpful at this time. > > None of that code should be used yet in a file with more than one > block, so I don't know how this could be causing problems. My plan was > that it would be up to the caller to seek to the next block based on > the returned data size. > Hmm. With the version that reads the CRC, I get more error messages and failures doing a 'make test' than the version without the addition. >> Second: At least two of the methods GuildWars2 wants to use are stubbed. >> To implement those methods, the frame section of the WIC object should >> contain an array (or something with similar properties) of pointers to >> chunks. From what I have read about WIC, other formats could use a >> similar array. Some could even use an array of pointers to frames. >> Before I go messing with that level of change, I think I should listen >> to other peoples opinions of the subject. > > Please keep in mind that Guild Wars 2 is probably not using WIC > directly. You should find out which library it's actually using > (probably d3dx9, but could also be gdiplus or oleaut32's IPicture > interface) and take that into account. We can modify users of WIC to > use it in efficient ways, and/or optimize WIC for the way we are using > it. (MSDN recommends reading the entire image once either in one > CopyPixels call or in rows from top to bottom, and optimizing for that > case. We don't do that yet because we needed the general case first.) > d3dx9. From comments from the developers, they were having trouble with slow access to their data file. It's a huge thing, 30+GB. (Yes, that is a G, not an M). I'd tried a patch on GetCount (which completely misunderstood) and it went on to do something 'ReaderByIndex'. It put more images on the screen much faster for some reason. I have very very little information on the game's internal structure other than what wine logs. >> Third: On a very broad level, the whole OLE implementation seems to be >> very messed up. In particular, the usual practice for doing >> inheritance in C does not seem to be being used. That practice is to >> have the elements common to the base and extending object be placed at >> the beginning of the implementing data structures. While the member >> names need not be the same in all instances, the function, type and >> order of the common elements must be the same for economical >> implementation. > > I don't think a base class would've make anything easier. > >> Is there any reason, other than inertia, that at least these two >> fields should not be moved to the beginning of implementing objects. >> This can be done one object at a time and would allow the changed >> objects to use a common implementation of 'AddRef' at least, and >> another common routine or macro that handles the critical part of >> 'Release'. > > We still couldn't do that because every interface vtable pointer would > be at a different offset within the struct. We couldn't share them > between encoders because different encoders have different data that > they need to free. Anyway, it's not likely to have a measurable impact > on performance. > OK. I was thinking every object based on IUnknown would start with a VTable pointer, a critical section lock and a reference count. I did not expect to see any performance improvement, just the absence of innumerable 'AddRef' and 'Release' variants. In fact I sort of expected both of them to go away as virtual functions with a virtual destructor in their place. > Could you share some results of your profiling? What is it that led > you to believe the PNG code is a bottleneck? > Not allowed to profile as such. It was just the difference in speed when the game thought it had 'PNGGetCount'

Oct. 24, 2012

Re: [PATCH 5/6] dsound: rework ugly mixer logic

by Chris Robinson

On 10/24/2012 05:16 AM, Maarten Lankhorst wrote: > I'm not sure yet how dsound should be fixed on bigger periods, I believe using > IAudioClock for reporting position *might* be the correct answer, but it's not > properly test yet on behavior, so I chose for the simple solution of directly > reporting next mix position as play pos, which is the same as current code. ... > At the point dsound queues the data to mmdevapi it can no longer do anything > about it, so instead of trying something more complicated the best way to deal > with it is to accept that fact and keep queued latency as fixed as possible, > and ideally small. FWIW, I agree. With OpenAL Soft, the "read" position of a sound is reported by simply using the next update position, and the "write" position is one period size ahead of that. The underlying output mix buffer is set to a fixed size with fixed update lengths (and where PulseAudio is concerned, makes sure to use PA_STREAM_ADJUST_LATENCY to keep its own latency in check). This seems to me to be more than adequate, even when using a custom dsound->openal wrapper.

Oct. 24, 2012

Re: windowscodecs questions.

by Vincent Povirk

> I found some code that reads a generic chunk. I see where the size > and type are read, where a correctly sized record is allocated and > where the data is read into the record. That code is unused for reading and writing PNG images right now, sorry for the confusion. Libpng has its own code for reading and writing PNG chunks, and that is what is being used. Our chunk code is used for PNG metadata handlers (which will have to work independently from libpng so we can have a proper, pluggable system for metadata), but the architecture isn't there yet to use those handlers when reading a PNG image. > I added code to read and check the CRC to my working copy of the git > tree. That threw off the block synchronization, so I think there is > code someplace else that either checks or skips the CRC, but I have > not found it after a few hours searching for it. An indication on > where to look and other advice would be very helpful at this time. None of that code should be used yet in a file with more than one block, so I don't know how this could be causing problems. My plan was that it would be up to the caller to seek to the next block based on the returned data size. > Second: At least two of the methods GuildWars2 wants to use are stubbed. > To implement those methods, the frame section of the WIC object should > contain an array (or something with similar properties) of pointers to > chunks. From what I have read about WIC, other formats could use a > similar array. Some could even use an array of pointers to frames. > Before I go messing with that level of change, I think I should listen > to other peoples opinions of the subject. Please keep in mind that Guild Wars 2 is probably not using WIC directly. You should find out which library it's actually using (probably d3dx9, but could also be gdiplus or oleaut32's IPicture interface) and take that into account. We can modify users of WIC to use it in efficient ways, and/or optimize WIC for the way we are using it. (MSDN recommends reading the entire image once either in one CopyPixels call or in rows from top to bottom, and optimizing for that case. We don't do that yet because we needed the general case first.) > Third: On a very broad level, the whole OLE implementation seems to be > very messed up. In particular, the usual practice for doing > inheritance in C does not seem to be being used. That practice is to > have the elements common to the base and extending object be placed at > the beginning of the implementing data structures. While the member > names need not be the same in all instances, the function, type and > order of the common elements must be the same for economical > implementation. I don't think a base class would've make anything easier. > Is there any reason, other than inertia, that at least these two > fields should not be moved to the beginning of implementing objects. > This can be done one object at a time and would allow the changed > objects to use a common implementation of 'AddRef' at least, and > another common routine or macro that handles the critical part of > 'Release'. We still couldn't do that because every interface vtable pointer would be at a different offset within the struct. We couldn't share them between encoders because different encoders have different data that they need to free. Anyway, it's not likely to have a measurable impact on performance. Could you share some results of your profiling? What is it that led you to believe the PNG code is a bottleneck?

Oct. 24, 2012

Re: [PATCH 1/5] d3dx9_36: Implement D3DXFileCreate. (try 3)

by Rico Schüller

On 24.10.2012 19:39, Nikolay Sivov wrote: > On 10/24/2012 19:02, Rico Schüller wrote: >> On 24.10.2012 16:33, Dmitry Timoshkov wrote: >>> Christian Costa <titan.costa(a)gmail.com> wrote: >>> >>>>>> +static HRESULT WINAPI ID3DXFileImpl_QueryInterface(ID3DXFile *iface, >>>>> REFIID riid, void **ret_iface) >>>>>> +{ >>>>>> + TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), >>>>>> ret_iface); >>>>>> + >>>>>> + if (IsEqualGUID(riid, &IID_IUnknown) || >>>>>> + IsEqualGUID(riid, &IID_ID3DXFile)) >>>>>> + { >>>>>> + iface->lpVtbl->AddRef(iface); >>>>> >>>>> Isn't there an appropriate xxx_AddRef() macro? >>>>> >>>> >>>> No. >>> >>> Is there a reason why? >>> >> Yes, they are not in the headers which are supplied by the SDK (at >> least not in the one I checked). I guess you have to ask ms why they >> didn't add it. So I think we shouldn't add that in our headers. > Actually I don't see why we can't add an .idl for it and let widl > produce a header. It still will be accessible directly through vtbl > pointer if someone feels like build with our headers. Header we're > talking about d3dx9xof.h is small enough to be a first converted to idl > imho, instead of adding more code that uses in SDK way. As an example we > have dwrite.idl now that SDK does not provide. > I have nothing against it, we just need to agree on something. Last time (http://www.winehq.org/pipermail/wine-devel/2010-June/084633.html) it was also criticized ... Do we care that some of our code eventually may be built with dxsdk headers? (That won't work for all dlls, but for some). How far do we go with compatibility? Cheers Rico

Oct. 24, 2012

Re: [PATCH 1/5] d3dx9_36: Implement D3DXFileCreate. (try 3)

by Nikolay Sivov

On 10/24/2012 19:02, Rico Schüller wrote: > On 24.10.2012 16:33, Dmitry Timoshkov wrote: >> Christian Costa <titan.costa(a)gmail.com> wrote: >> >>>>> +static HRESULT WINAPI ID3DXFileImpl_QueryInterface(ID3DXFile *iface, >>>> REFIID riid, void **ret_iface) >>>>> +{ >>>>> + TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), >>>>> ret_iface); >>>>> + >>>>> + if (IsEqualGUID(riid, &IID_IUnknown) || >>>>> + IsEqualGUID(riid, &IID_ID3DXFile)) >>>>> + { >>>>> + iface->lpVtbl->AddRef(iface); >>>> >>>> Isn't there an appropriate xxx_AddRef() macro? >>>> >>> >>> No. >> >> Is there a reason why? >> > Yes, they are not in the headers which are supplied by the SDK (at > least not in the one I checked). I guess you have to ask ms why they > didn't add it. So I think we shouldn't add that in our headers. Actually I don't see why we can't add an .idl for it and let widl produce a header. It still will be accessible directly through vtbl pointer if someone feels like build with our headers. Header we're talking about d3dx9xof.h is small enough to be a first converted to idl imho, instead of adding more code that uses in SDK way. As an example we have dwrite.idl now that SDK does not provide.

Oct. 24, 2012