Le 27/05/2013 09:22, Henri Verbeet a écrit :
dlls/d3drm/d3drm.c | 20 ++++++++------------ dlls/d3drm/frame.c | 22 +++++++--------------- dlls/d3drm/meshbuilder.c | 14 +++++++------- dlls/d3drm/tests/d3drm.c | 16 ++++++++-------- include/d3drm.h | 4 ++-- include/d3drmobj.h | 26 +++++++++++++------------- 6 files changed, 45 insertions(+), 57 deletions(-)
diff --git a/dlls/d3drm/d3drm.c b/dlls/d3drm/d3drm.c index 8dfa4bb..fe728a8 100644 --- a/dlls/d3drm/d3drm.c +++ b/dlls/d3drm/d3drm.c @@ -317,24 +317,20 @@ static HRESULT WINAPI IDirect3DRMImpl_CreateUserVisual(IDirect3DRM* iface, D3DRM return E_NOTIMPL; }
-static HRESULT WINAPI IDirect3DRMImpl_LoadTexture(IDirect3DRM* iface, const char* filename,
LPDIRECT3DRMTEXTURE* Texture)+static HRESULT WINAPI IDirect3DRMImpl_LoadTexture(IDirect3DRM *iface,
const char *filename, IDirect3DRMTexture **texture)
- IDirect3DRMImpl *This = impl_from_IDirect3DRM(iface);
- FIXME("(%p/%p)->(%s,%p): stub\n", iface, This, filename, Texture);
- FIXME("iface %p, filename %s, texture %p stub!\n", iface, debugstr_a(filename), texture);
Do you really need to change the way traces are displayed?
On 27 May 2013 09:49, Christian Costa titan.costa@gmail.com wrote:
- FIXME("(%p/%p)->(%s,%p): stub\n", iface, This, filename, Texture);
- FIXME("iface %p, filename %s, texture %p stub!\n", iface,
debugstr_a(filename), texture);
Do you really need to change the way traces are displayed?
Yes, you can't use %s for arbitrary application data. The existing traces are also ugly. Arguably that could have been a separate patch, but I'm touching the line anyway because the variable name changed.
2013/5/27 Henri Verbeet hverbeet@gmail.com
On 27 May 2013 09:49, Christian Costa titan.costa@gmail.com wrote:
- FIXME("(%p/%p)->(%s,%p): stub\n", iface, This, filename, Texture);
- FIXME("iface %p, filename %s, texture %p stub!\n", iface,
debugstr_a(filename), texture);
Do you really need to change the way traces are displayed?
Yes, you can't use %s for arbitrary application data. The existing traces are also ugly. Arguably that could have been a separate patch, but I'm touching the line anyway because the variable name changed.
No problem with debugstr_a nor variable rename. I meant the format string. The uglyness seems to be a matter of taste. It is used in many places in wine for COM objects. So unless there is a global will to change traces (like LP stuff removal), that would be better to keep traces as they are for consistency with the rest of the code.
On 27 May 2013 11:14, Christian Costa titan.costa@gmail.com wrote:
So unless there is a global will to change traces (like LP stuff removal), that would be better to keep traces as they are for consistency with the rest of the code.
This makes it consistent with most other D3D code, like wined3d, dxgi, d3d10core, d3d10, d3d9, d3d8, ddraw, d3dcompiler, and to some extent d3dx9.
This is not a good argument. By keeping the current format, it stays consistent with d3drm, d3dxof, quartz, strmbase, devenum, amstream, dinput, dplay, direct music and d3dx9 in some extents and others. Original ddraw/d3d used the same format as d3drm until you changed it (as well as methods names). Since you're working on it that's not a problem but there is no need to change other dlls if you're not working on them. If consistency in wine is something that matters, and I think it does, discussion is a better way for that.
2013/5/27 Henri Verbeet hverbeet@gmail.com
On 27 May 2013 11:14, Christian Costa titan.costa@gmail.com wrote:
So unless there is a global will to change traces (like LP stuff
removal),
that would be better to keep traces as they are for consistency with the rest of the code.
This makes it consistent with most other D3D code, like wined3d, dxgi, d3d10core, d3d10, d3d9, d3d8, ddraw, d3dcompiler, and to some extent d3dx9.
On 27 May 2013 13:19, Christian Costa titan.costa@gmail.com wrote:
Original ddraw/d3d used the same format as d3drm until you changed it (as well as methods names).
This is perhaps slightly bad form, but since you're pressing the point: Yes, and those were hardly the only things that were wrong with it.
Since you're working on it that's not a problem but there is no need to change other dlls if you're not working on them.
Clearly I'm working on it now. However, if what you're actually saying is that you're claiming maintainership over d3drm that's ok, but then I'd also like to see some actual maintenance.
2013/5/27 Henri Verbeet hverbeet@gmail.com
On 27 May 2013 13:19, Christian Costa titan.costa@gmail.com wrote:
Original ddraw/d3d used the same format as d3drm until you changed it (as well as methods names).
This is perhaps slightly bad form, but since you're pressing the point: Yes, and those were hardly the only things that were wrong with it.
This is your point of view.
Since you're working on it that's not a problem but there is no need to change other dlls if
you're
not working on them.
Clearly I'm working on it now. However, if what you're actually saying is that you're claiming maintainership over d3drm that's ok, but then I'd also like to see some actual maintenance.
André and I have been working on it so far when needed so yes. I'm wondering what work you're doing. So far I've only seen some cleanup like LP stuff removal. That would be better to see real code before cleaning the code to suit your taste. And anyway if I had to work again on d3drm after you changed all the traces or whatever, should I change all traces back to something I prefer because I'm working on it, forgetting people that may have recently worked or still working on it? If you think your traces are better that would be better to convince people and put something in the coding guideline. Currently there is no parenthesis whereas all code in wine use them for tracing function call and this make log lines longer which some was arguing it was a bad thing.
-
Christian Costa -
Henri Verbeet