Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Am Dienstag 28 November 2006 22:05 schrieb Markus Amsler: Stefan Dösinger wrote: I proposed getting rid of COM some time ago, but I didn't consider that we need some features like inheritance. I think we should keep COM, but get rid of the D3D-Specific Addrefs because they are different between the various versions d3d versions. So keep the AddRef in IUnknown methods(QueryInterface namely), and GetParent, but do not AddRef in d3d methods(SetTexture / GetTexture, ...) Opinions? From my point of view (solving d3dx implicit surface refcounts) the GetParent AddRef is ugly. It spreads d3dx refcounting across d3dx and wined3d. It would be easier to only do d3dx refcounting in d3dx. But that's only my pragmatic view, without further COM knowledge. Well, GetParent isn't part of IUnknown(its a wined3d invention), so from the COM point of view we're fine to do whatever we want with it. (Just QueryInterface has the rule that it addrefs the returned interface) pgpI5qimwsu1l.pgp Description: PGP signature
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Stefan Dösinger [EMAIL PROTECTED] writes: Well, GetParent isn't part of IUnknown(its a wined3d invention), so from the COM point of view we're fine to do whatever we want with it. (Just QueryInterface has the rule that it addrefs the returned interface) The COM rules are not limited to QueryInterface, any call that returns an interface pointer needs to addref it. -- Alexandre Julliard [EMAIL PROTECTED]
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
On 28/11/06, Markus Amsler [EMAIL PROTECTED] wrote: One problem is, half of the AddRef patches were commited. Before reverting them, I thought I send the rest again. The other problem is the AddRef in GetParent is ugly, because it AddRefs on a d3dx object. We should do d3dx refcounting only in d3dx. At least this one has to go, or the implicit surface refcount code gets ugly. That was why i removed all of them, to be consistent with GetParent. Well, no. The basic rule there is that when a function produces a reference to an interface, it should AddRef that interface. As for the previous patches, I was under the impression at the time that the idea was to get rid of COM in wined3d, otherwise I would have said something earlier. For consistency it would probably be best if they were reverted. And to be consistent with other COM objects (like ddraw, d3d8, d3d9), we would have to add some more ugly hacks :-) (like not destroying on count 0, forward refcount of one object to another, ...) D3D does have some weird reference behaviour, but those should be the exceptions, not a reason to completely violate COM rules in the rest of the code.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
H. Verbeet [EMAIL PROTECTED] writes: As for the previous patches, I was under the impression at the time that the idea was to get rid of COM in wined3d, otherwise I would have said something earlier. For consistency it would probably be best if they were reverted. That was my impression too, what happened to the idea of getting rid of COM? Because of course if we are keeping COM then we need to follow the rules properly. -- Alexandre Julliard [EMAIL PROTECTED]
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
On 28/11/06, Alexandre Julliard [EMAIL PROTECTED] wrote: H. Verbeet [EMAIL PROTECTED] writes: As for the previous patches, I was under the impression at the time that the idea was to get rid of COM in wined3d, otherwise I would have said something earlier. For consistency it would probably be best if they were reverted. That was my impression too, what happened to the idea of getting rid of COM? Because of course if we are keeping COM then we need to follow the rules properly. Afaik the idea is still around, but nobody is actively working on it. Although getting rid of COM could potentially simplify wined3d a bit, for me personally it isn't much of a priority. One concern I have about getting rid of COM is that it has the potential to cause quite a few regressions, not all of which will be immediately obvious.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Alexandre Julliard wrote: H. Verbeet [EMAIL PROTECTED] writes: As for the previous patches, I was under the impression at the time that the idea was to get rid of COM in wined3d, otherwise I would have said something earlier. For consistency it would probably be best if they were reverted. That was my impression too, what happened to the idea of getting rid of COM? Because of course if we are keeping COM then we need to follow the rules properly. I wasn't fully aware of the importance of the COM refcount rules, just saw the opportunity for simplification. For removing COM it would be probably better to start with removing the lpVtbl's. I agree it's probably best to revert them for consistency. Should I send patches? Sorry for the inconveniences.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Markus Amsler [EMAIL PROTECTED] writes: I wasn't fully aware of the importance of the COM refcount rules, just saw the opportunity for simplification. For removing COM it would be probably better to start with removing the lpVtbl's. I agree it's probably best to revert them for consistency. Should I send patches? If we are not going further with the COM removal, then yes if you could send patches to revert the changes that would be good. -- Alexandre Julliard [EMAIL PROTECTED]
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Am Dienstag 28 November 2006 16:55 schrieb Alexandre Julliard: Markus Amsler [EMAIL PROTECTED] writes: I wasn't fully aware of the importance of the COM refcount rules, just saw the opportunity for simplification. For removing COM it would be probably better to start with removing the lpVtbl's. I agree it's probably best to revert them for consistency. Should I send patches? If we are not going further with the COM removal, then yes if you could send patches to revert the changes that would be good. I proposed getting rid of COM some time ago, but I didn't consider that we need some features like inheritance. I think we should keep COM, but get rid of the D3D-Specific Addrefs because they are different between the various versions d3d versions. So keep the AddRef in IUnknown methods(QueryInterface namely), and GetParent, but do not AddRef in d3d methods(SetTexture / GetTexture, ...) Opinions? pgplNtj0GBass.pgp Description: PGP signature
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Stefan Dösinger wrote: I proposed getting rid of COM some time ago, but I didn't consider that we need some features like inheritance. I think we should keep COM, but get rid of the D3D-Specific Addrefs because they are different between the various versions d3d versions. So keep the AddRef in IUnknown methods(QueryInterface namely), and GetParent, but do not AddRef in d3d methods(SetTexture / GetTexture, ...) Opinions? From my point of view (solving d3dx implicit surface refcounts) the GetParent AddRef is ugly. It spreads d3dx refcounting across d3dx and wined3d. It would be easier to only do d3dx refcounting in d3dx. But that's only my pragmatic view, without further COM knowledge.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
H. Verbeet wrote: On 28/11/06, Markus Amsler [EMAIL PROTECTED] wrote: The following patches remove refcounting in wined3d Getters. The Setters/Creaters refcounting can't be removed right now, because of the way implicit surfaces are currently handled. The idea is to simplify the d3dx-wined3d refcount relation. Ideally every wined3d object gets created with refcount=1, never AddRef'ed and destroyed with the first Release call. Rob mentioned this violates COM rules, but wined3d is wine internal. The GetParent patch doesn't remove wined3d refcounting, it seperates d3dx and wined3d refcounting (Which was the reason I started to remove AddRefs). --- I think this is a bad idea, if only for consistency with other COM objects. I don't think wined3d being wine internal is a very good reason to violate COM rules. IMO if we're going to use COM we should stick to its rules. Also see my other mail. One problem is, half of the AddRef patches were commited. Before reverting them, I thought I send the rest again. The other problem is the AddRef in GetParent is ugly, because it AddRefs on a d3dx object. We should do d3dx refcounting only in d3dx. At least this one has to go, or the implicit surface refcount code gets ugly. That was why i removed all of them, to be consistent with GetParent. And to be consistent with other COM objects (like ddraw, d3d8, d3d9), we would have to add some more ugly hacks :-) (like not destroying on count 0, forward refcount of one object to another, ...)
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
Markus Amsler wrote: @@ -4704,7 +4704,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl TRACE((%p) : ppDecl=%p\n, This, ppDecl); *ppDecl = This-stateBlock-vertexDecl; -if (NULL != *ppDecl) IWineD3DVertexDeclaration_AddRef(*ppDecl); return WINED3D_OK; These patches violate COM rules. Is this for performance or other reasons? -- Rob Shearman
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
On 22/11/06, Robert Shearman [EMAIL PROTECTED] wrote: Markus Amsler wrote: @@ -4704,7 +4704,6 @@ static HRESULT WINAPI IWineD3DDeviceImpl TRACE((%p) : ppDecl=%p\n, This, ppDecl); *ppDecl = This-stateBlock-vertexDecl; -if (NULL != *ppDecl) IWineD3DVertexDeclaration_AddRef(*ppDecl); return WINED3D_OK; These patches violate COM rules. Is this for performance or other reasons? The plan seems to be to not use COM for wined3d anymore, mainly because the layering of d3d87/8/9 on top of wined3d is causing some problems with reference counts. Those could probably be solved by using eg aggregation, but I think the general feeling is that COM only makes things harder in wined3d.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
H. Verbeet wrote: On 22/11/06, Robert Shearman [EMAIL PROTECTED] wrote: These patches violate COM rules. Is this for performance or other reasons? The plan seems to be to not use COM for wined3d anymore, mainly because the layering of d3d87/8/9 on top of wined3d is causing some problems with reference counts. Those could probably be solved by using eg aggregation, but I think the general feeling is that COM only makes things harder in wined3d. It's about simplifying code, with the side effect of perhaps small performance gains. It's also the first step to drop COM in wined3d, but that's not my intention (it seems Stefan likes the idea). Wined3d refcounting is not causing refcount problems in d3dx, they are completly different. It just makes things easier.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
On 22/11/06, Markus Amsler [EMAIL PROTECTED] wrote: It's about simplifying code, with the side effect of perhaps small performance gains. It's also the first step to drop COM in wined3d, but that's not my intention (it seems Stefan likes the idea). Well, you can't half-remove COM. If you start removing refcounts like this you have to follow it all the way through, imo.
Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
H. Verbeet wrote: On 22/11/06, Markus Amsler [EMAIL PROTECTED] wrote: It's about simplifying code, with the side effect of perhaps small performance gains. It's also the first step to drop COM in wined3d, but that's not my intention (it seems Stefan likes the idea). Well, you can't half-remove COM. If you start removing refcounts like this you have to follow it all the way through, imo. I'm not half removing COM (at most 3% :-) ). lpVtble, IUnknown, IID_*, ... is still in place. I just simplified some refcount behaviour in wined3d, which happens to make wined3d less dependandt on the refcount COM feature. Which in turn makes it easier to remove COM. I dont' dare even trying to remove COM. That would be far over my skills.
