Re: d3d [1]: Remove AddRef from IWineD3DDevice_GetVertexDeclaration.
On 28/11/06, Markus Amsler <markus.amsler(a)oribi.org> 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.
H. Verbeet wrote:
On 28/11/06, Markus Amsler <markus.amsler(a)oribi.org> 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, ...)
On 28/11/06, Markus Amsler <markus.amsler(a)oribi.org> 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.
"H. Verbeet" <hverbeet(a)gmail.com> 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 julliard(a)winehq.org
On 28/11/06, Alexandre Julliard <julliard(a)winehq.org> wrote:
"H. Verbeet" <hverbeet(a)gmail.com> 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.
Alexandre Julliard wrote:
"H. Verbeet" <hverbeet(a)gmail.com> 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.
Markus Amsler <markus.amsler(a)oribi.org> 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 julliard(a)winehq.org
Am Dienstag 28 November 2006 16:55 schrieb Alexandre Julliard:
Markus Amsler <markus.amsler(a)oribi.org> 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?
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.
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)
Stefan Dösinger <stefandoesinger(a)gmx.at> 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 julliard(a)winehq.org
participants (4)
-
Alexandre Julliard -
H. Verbeet -
Markus Amsler -
Stefan Dösinger