Re: Patch 85420
2012/4/17 Michael Stefaniuc > On 04/17/2012 09:46 PM, Marcus Meissner wrote: > > On Tue, Apr 17, 2012 at 09:10:45PM +0200, Christian Costa wrote: > >> Patch http://source.winehq.org/patches/data/85420 is marked as > >> pending. I've taken a look but I still don't understand what's > >> wrong. > >> Is there anything I've missed ? > > > > I stopped reading at this line: > > > > +*frame = > (LPDIRECT3DRMFRAME)(&impl_from_IDirect3DRMFrame3(This->parent)->IDirect3DRMFrame2_iface); > > > > You are doing something very wrong if you need to cast this way. > That's just the stupid obfuscation introduced by using LPDIRECT3DRMFRAME > instead of (IDirect3DRMFrame *) and then the cast might be plausible > (e.g. a function taking an IFace* type but which is implemented by the > IFace2 interface). > > Yes that's exactly the case. > There are three other issues with that line though that kill it: > - For type safety reasons impl_from_IFace() needs to be used *only* in > variable declarations. > Ok. Does not seem to be explicit on the COM guidelines. Could you make it more explicit on the wiki ? > - Calling impl_from_IFace() on interface pointers outside methods that > implement that interface are verboten. > Same as above. > - Both above points can be completely avoided by making the parent field > be "IDirect3DRMFrameImpl *parent". > Ok. I will do that. > > Christian
Re: Patch 85420
On 04/17/2012 09:46 PM, Marcus Meissner wrote: > On Tue, Apr 17, 2012 at 09:10:45PM +0200, Christian Costa wrote: >> Patch http://source.winehq.org/patches/data/85420 is marked as >> pending. I've taken a look but I still don't understand what's >> wrong. >> Is there anything I've missed ? > > I stopped reading at this line: > > +*frame = > (LPDIRECT3DRMFRAME)(&impl_from_IDirect3DRMFrame3(This->parent)->IDirect3DRMFrame2_iface); > > You are doing something very wrong if you need to cast this way. That's just the stupid obfuscation introduced by using LPDIRECT3DRMFRAME instead of (IDirect3DRMFrame *) and then the cast might be plausible (e.g. a function taking an IFace* type but which is implemented by the IFace2 interface). There are three other issues with that line though that kill it: - For type safety reasons impl_from_IFace() needs to be used *only* in variable declarations. - Calling impl_from_IFace() on interface pointers outside methods that implement that interface are verboten. - Both above points can be completely avoided by making the parent field be "IDirect3DRMFrameImpl *parent". > It probably also needs explicit sign-off drom the d3d folks as it handles > funny refcounting. Not really, this is plain COM stuff. bye michael
Re: Patch 85420
On Tue, Apr 17, 2012 at 09:10:45PM +0200, Christian Costa wrote: > Hi, > > Patch http://source.winehq.org/patches/data/85420 is marked as > pending. I've taken a look but I still don't understand what's > wrong. > Is there anything I've missed ? I stopped reading at this line: +*frame = (LPDIRECT3DRMFRAME)(&impl_from_IDirect3DRMFrame3(This->parent)->IDirect3DRMFrame2_iface); You are doing something very wrong if you need to cast this way. It probably also needs explicit sign-off drom the d3d folks as it handles funny refcounting. Ciao, Marcus
Patch 85420
Hi, Patch http://source.winehq.org/patches/data/85420 is marked as pending. I've taken a look but I still don't understand what's wrong. Is there anything I've missed ? Thanks Christian
