Re: [1/5] wined3d: Don't check the FBO status if FIXMEs are off

2011-05-25 Thread Stefan Dösinger
On Tuesday 24 May 2011 22:57:34 Henri Verbeet wrote:
 I'm not sure that's really worth it. If the GL lock is really such a
 concern, you should probably change the calls in context.c to already
 be under the GL lock anyway. You can easily change
 context_apply_draw_buffers() for example to require locking by the
 caller.
I planned to do this on top of this patch, but it turned out to be way easier 
than expected. Please ignore this patch for now. The others should still apply 
though.


signature.asc
Description: This is a digitally signed message part.



Re: GSoC-2011: Implement Missing Mesh Functions in Wine’s D3DX9

2011-05-25 Thread Michael Mc Donnell
On Tue, May 24, 2011 at 8:32 PM, Stefan Dösinger stefandoesin...@gmx.at wrote:
 On Tuesday 24 May 2011 19:56:06 Michael Mc Donnell wrote:

 Sorry I phrased that in a wrong way. In the UpdateSemantics function I
 call IDirect3DDevice9_CreateVertexDeclaration which allocates a new
 Vertex Declaration on the heap.
 Is there an alternative? I guess you could cache existing declarations. The
 d3d9 API doesn't give you an option to reuse an existing declaration.

 I doubt apps will regularly change the declaration, usually this doesn't make
 sense without also changing the data. Of course that doesn't mean there isn't
 a broken app out there that does this.

Yeah it might not be a problem at all. I'll give it a quick second
look to see if it's possible to cache things. I anyway need to read
some more code too to see exactly how the APIs interact.

 I also noticed that I didn't call
 IDirect3DDevice9_SetVertexDeclaration. I've added it to stick with the
 defined locking scheme.
 Locking scheme? I've just started reading into into the Mesh API, but I don't
 think you're supposed to apply the declaration until the mesh is used for
 drawing.

Ok good, I started to doubt this last night. I'll remove it and try to
get a better sense of how the API works.

  *) In your first test you forgot to check the HeapAlloc result.

 Ok, I'll return E_OUTOFMEMORY in that case.
 I guess the callers will only abort the tests if NewTestContext fails, so I
 think the NULL / non-NULL return value was better.

 Should I try to make some more invalid
 D3DVERTEXELEMENT9 arrays to see if I can provoke an error?
 I was thinking about something that would make CreateVertexDeclaration fail,
 e.g. declaring a usage+usage index twice or using an undefined type.
 dlls/d3d9/vertexdeclaration.c and dlls/wined3d/vertexdeclaration.c check for a
 few error conditions in their vertexdeclaration_init functions. Just pick one
 of them(no need to check all of them)
 But watch out that you don't open a can of worms here. Our
 CreateVertexDeclaration probably doesn't catch all error conditions. I
 recommend that you pick one it does catch, otherwise you'll have to fix
 d3d9.dll and wined3d.dll too for a small test.

Ok thanks I'll do that.

  *) In the wined3d code(and its client libs) Henri and I avoid structure
  typedefs. The existing d3dx9 code has a few of them. I'm ok with either
  way, but maybe you and the other d3dx9 devs want to go the wined3d way.

 Sure I can change them. So just normal structs? Is it to keep the
 namespace clean?
 Henri started with this change, I think keeping the namespace clean was a
 consideration. What you do is up to you and the other devs working on d3dx9.
 I'm only suggesting that you discuss the code style and agree on a format.

 Either way it is a fairly minor point IMO, but it tends to end up in holy war.

Ok :-) I don't have any strong feelings about it, so I'll just follow
your convention until somebody complains.




Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

2011-05-25 Thread Michael Stefaniuc
Hello Ričardas!

Ričardas Barkauskas wrote:
 My first try for COM cleanup. Please donėt be too harsh :)
 If this is too large for one patch, please suggest how to split it.
 
 This is not automatic clean up so I don't really expect it to be good on
 first try.
Heh! I have attached the raw diff generated by my coccinelle script.
Looking at the interdiff between the two patches it looks like you
caught everything. And the BADBADBAD() (*) from the generated patch are
replaced with unsafe_impl_from_IDirectDrawSurface3(), very good.

If you want to do more COM cleanup I can give you the raw generated
patches to save you some time.

bye
michael

P.S.: (*) My script validates that impl_from_IFace() is called only from
methods of that IFace. All other impl_from_IFace() calls are replaced
with BADBADBAD().
diff -u -p a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
--- a/dlls/ddraw/ddraw_private.h 2011-05-24 21:52:30.191863832 +0200
+++ b/dlls/ddraw/ddraw_private.h 2011-05-24 22:04:40.459493227 +0200
@@ -279,7 +279,7 @@ struct IDirect3DDeviceImpl
 {
 /* IUnknown */
 const IDirect3DDevice7Vtbl *lpVtbl;
-const IDirect3DDevice3Vtbl *IDirect3DDevice3_vtbl;
+IDirect3DDevice3 IDirect3DDevice3_iface;
 const IDirect3DDevice2Vtbl *IDirect3DDevice2_vtbl;
 const IDirect3DDeviceVtbl *IDirect3DDevice_vtbl;
 LONGref;
@@ -336,9 +336,9 @@ static inline IDirect3DDeviceImpl *devic
 return (IDirect3DDeviceImpl *)((char*)iface - FIELD_OFFSET(IDirect3DDeviceImpl, IDirect3DDevice2_vtbl));
 }
 
-static inline IDirect3DDeviceImpl *device_from_device3(IDirect3DDevice3 *iface)
+static inline IDirect3DDeviceImpl *impl_from_IDirect3DDevice3(IDirect3DDevice3 *iface)
 {
-return (IDirect3DDeviceImpl *)((char*)iface - FIELD_OFFSET(IDirect3DDeviceImpl, IDirect3DDevice3_vtbl));
+return CONTAINING_RECORD(iface, IDirect3DDeviceImpl, IDirect3DDevice3_iface);
 }
 
 /* Structures */
diff -u -p a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
--- a/dlls/ddraw/ddraw.c 2011-05-24 20:51:49.671810005 +0200
+++ b/dlls/ddraw/ddraw.c 2011-05-24 22:04:40.569494647 +0200
@@ -4763,7 +4763,7 @@ static HRESULT WINAPI d3d3_CreateDevice(
 
 hr = d3d7_CreateDevice(This-IDirect3D7_iface, riid, (IDirectDrawSurface7 *)surface,
 (IDirect3DDevice7 **)device);
-if (*device) *device = (IDirect3DDevice3 *)((IDirect3DDeviceImpl *)*device)-IDirect3DDevice3_vtbl;
+if (*device) *device = (IDirect3DDevice3 *)(BADBADBAD(*device))-IDirect3DDevice3_vtbl;
 
 return hr;
 }
diff -u -p a/dlls/ddraw/device.c b/dlls/ddraw/device.c
--- a/dlls/ddraw/device.c 2011-05-24 20:51:58.111927504 +0200
+++ b/dlls/ddraw/device.c 2011-05-24 22:04:42.142848293 +0200
@@ -171,7 +171,7 @@ IDirect3DDeviceImpl_7_QueryInterface(IDi
 TRACE((%p) Returning IDirect3DDevice2 interface at %p\n, This, *obj);
 }
 else if ( IsEqualGUID( IID_IDirect3DDevice3  , refiid ) ) {
-*obj = This-IDirect3DDevice3_vtbl;
+*obj = This-IDirect3DDevice3_iface;
 TRACE((%p) Returning IDirect3DDevice3 interface at %p\n, This, *obj);
 }
 else if ( IsEqualGUID( IID_IDirect3DDevice7  , refiid ) ) {
@@ -196,7 +196,7 @@ static HRESULT WINAPI IDirect3DDeviceImp
 {
 TRACE(iface %p, riid %s, object %p.\n, iface, debugstr_guid(riid), obj);
 
-return IDirect3DDevice7_QueryInterface((IDirect3DDevice7 *)device_from_device3(iface), riid, obj);
+return IDirect3DDevice7_QueryInterface((IDirect3DDevice7 *)impl_from_IDirect3DDevice3(iface), riid, obj);
 }
 
 static HRESULT WINAPI IDirect3DDeviceImpl_2_QueryInterface(IDirect3DDevice2 *iface, REFIID riid,
@@ -242,7 +242,7 @@ static ULONG WINAPI IDirect3DDeviceImpl_
 {
 TRACE(iface %p.\n, iface);
 
-return IDirect3DDevice7_AddRef((IDirect3DDevice7 *)device_from_device3(iface));
+return IDirect3DDevice7_AddRef((IDirect3DDevice7 *)impl_from_IDirect3DDevice3(iface));
 }
 
 static ULONG WINAPI IDirect3DDeviceImpl_2_AddRef(IDirect3DDevice2 *iface)
@@ -378,7 +378,7 @@ static ULONG WINAPI IDirect3DDeviceImpl_
 {
 TRACE(iface %p.\n, iface);
 
-return IDirect3DDevice7_Release((IDirect3DDevice7 *)device_from_device3(iface));
+return IDirect3DDevice7_Release((IDirect3DDevice7 *)impl_from_IDirect3DDevice3(iface));
 }
 
 static ULONG WINAPI IDirect3DDeviceImpl_2_Release(IDirect3DDevice2 *iface)
@@ -497,7 +497,7 @@ IDirect3DDeviceImpl_3_GetCaps(IDirect3DD
   D3DDEVICEDESC *HWDesc,
   D3DDEVICEDESC *HelDesc)
 {
-IDirect3DDeviceImpl *This = device_from_device3(iface);
+IDirect3DDeviceImpl *This = impl_from_IDirect3DDevice3(iface);
 D3DDEVICEDESC7 newDesc;
 HRESULT hr;
 
@@ -515,7 +515,7 @@ static HRESULT WINAPI IDirect3DDeviceImp
 {
 IDirect3DDeviceImpl *This = device_from_device2(iface);
 TRACE(iface %p, hw_desc %p, hel_desc %p.\n, iface, D3DHWDevDesc, D3DHELDevDesc);
-return IDirect3DDevice3_GetCaps((IDirect3DDevice3 *)This-IDirect3DDevice3_vtbl, 

Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

2011-05-25 Thread Michael Stefaniuc
Damn ... I have attached the wrong diff.

Michael Stefaniuc wrote:
 Ričardas Barkauskas wrote:
 My first try for COM cleanup. Please donėt be too harsh :)
 If this is too large for one patch, please suggest how to split it.

 This is not automatic clean up so I don't really expect it to be good on
 first try.
 Heh! I have attached the raw diff generated by my coccinelle script.
 Looking at the interdiff between the two patches it looks like you
 caught everything. And the BADBADBAD() (*) from the generated patch are
 replaced with unsafe_impl_from_IDirectDrawSurface3(), very good.
 
 If you want to do more COM cleanup I can give you the raw generated
 patches to save you some time.
 
 bye
   michael
 
 P.S.: (*) My script validates that impl_from_IFace() is called only from
 methods of that IFace. All other impl_from_IFace() calls are replaced
 with BADBADBAD().
diff -u -p a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
--- a/dlls/ddraw/ddraw_private.h 2011-05-24 21:52:30.191863832 +0200
+++ b/dlls/ddraw/ddraw_private.h 2011-05-24 22:03:50.322175271 +0200
@@ -168,7 +168,7 @@ struct IDirectDrawSurfaceImpl
 {
 /* IUnknown fields */
 const IDirectDrawSurface7Vtbl *lpVtbl;
-const IDirectDrawSurface3Vtbl *IDirectDrawSurface3_vtbl;
+IDirectDrawSurface3 IDirectDrawSurface3_iface;
 const IDirectDrawGammaControlVtbl *IDirectDrawGammaControl_vtbl;
 const IDirect3DTexture2Vtbl *IDirect3DTexture2_vtbl;
 const IDirect3DTextureVtbl *IDirect3DTexture_vtbl;
@@ -235,9 +235,9 @@ static inline IDirectDrawSurfaceImpl *su
 return (IDirectDrawSurfaceImpl *)((char*)iface - FIELD_OFFSET(IDirectDrawSurfaceImpl, IDirect3DTexture2_vtbl));
 }
 
-static inline IDirectDrawSurfaceImpl *surface_from_surface3(IDirectDrawSurface3 *iface)
+static inline IDirectDrawSurfaceImpl *impl_from_IDirectDrawSurface3(IDirectDrawSurface3 *iface)
 {
-return (IDirectDrawSurfaceImpl *)((char*)iface - FIELD_OFFSET(IDirectDrawSurfaceImpl, IDirectDrawSurface3_vtbl));
+return CONTAINING_RECORD(iface, IDirectDrawSurfaceImpl, IDirectDrawSurface3_iface);
 }
 
 /* Get the number of bytes per pixel for a given surface */
diff -u -p a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
--- a/dlls/ddraw/ddraw.c 2011-05-24 20:51:49.671810005 +0200
+++ b/dlls/ddraw/ddraw.c 2011-05-24 22:03:50.428843331 +0200
@@ -2023,7 +2023,7 @@ static HRESULT WINAPI ddraw3_GetGDISurfa
 TRACE(iface %p, surface %p.\n, iface, surface);
 
 hr = ddraw7_GetGDISurface(This-IDirectDraw7_iface, surface7);
-*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_vtbl : NULL;
+*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_iface : NULL;
 
 return hr;
 }
@@ -2037,7 +2037,7 @@ static HRESULT WINAPI ddraw2_GetGDISurfa
 TRACE(iface %p, surface %p.\n, iface, surface);
 
 hr = ddraw7_GetGDISurface(This-IDirectDraw7_iface, surface7);
-*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_vtbl : NULL;
+*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_iface : NULL;
 
 return hr;
 }
@@ -2051,7 +2051,7 @@ static HRESULT WINAPI ddraw1_GetGDISurfa
 TRACE(iface %p, surface %p.\n, iface, surface);
 
 hr = ddraw7_GetGDISurface(This-IDirectDraw7_iface, surface7);
-*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_vtbl : NULL;
+*surface = surface7 ? (IDirectDrawSurface *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_iface : NULL;
 
 return hr;
 }
@@ -2416,7 +2416,7 @@ static HRESULT WINAPI ddraw4_GetSurfaceF
 if (!surface) return E_INVALIDARG;
 
 hr = ddraw7_GetSurfaceFromDC(This-IDirectDraw7_iface, dc, surface7);
-*surface = surface7 ? (IDirectDrawSurface4 *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_vtbl : NULL;
+*surface = surface7 ? (IDirectDrawSurface4 *)((IDirectDrawSurfaceImpl *)surface7)-IDirectDrawSurface3_iface : NULL;
 
 return hr;
 }
@@ -3508,7 +3508,7 @@ static HRESULT WINAPI ddraw3_CreateSurfa
 }
 
 impl = (IDirectDrawSurfaceImpl *)surface7;
-*surface = (IDirectDrawSurface *)impl-IDirectDrawSurface3_vtbl;
+*surface = (IDirectDrawSurface *)impl-IDirectDrawSurface3_iface;
 ddraw_set_surface_version(impl, 3);
 IDirectDraw7_Release(This-IDirectDraw7_iface);
 IDirectDraw3_AddRef(iface);
@@ -3554,7 +3554,7 @@ static HRESULT WINAPI ddraw2_CreateSurfa
 }
 
 impl = (IDirectDrawSurfaceImpl *)surface7;
-*surface = (IDirectDrawSurface *)impl-IDirectDrawSurface3_vtbl;
+*surface = (IDirectDrawSurface *)impl-IDirectDrawSurface3_iface;
 ddraw_set_surface_version(impl, 2);
 IDirectDraw7_Release(This-IDirectDraw7_iface);
 impl-ifaceToRelease = NULL;
@@ -3590,7 +3590,7 @@ static HRESULT WINAPI ddraw1_CreateSurfa
 }
 
 impl = (IDirectDrawSurfaceImpl *)surface7;
-   

Re: usp10/tests: Make enumFontProc() static.

2011-05-25 Thread Marvin
Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
http://testbot.winehq.org/JobDetails.pl?Key=11170

Your paranoid android.


=== WNT4WSSP6 (32 bit usp10) ===
Timeout




Re: [PATCH] version: mark internal function with hidden visibility

2011-05-25 Thread Alexandre Julliard
Marcus Meissner meiss...@suse.de writes:

 @@ -348,7 +348,7 @@ static BOOL find_pe_resource( HFILE lzfd, DWORD *resLen, 
 DWORD *resOff )
  /***
   *   find_version_resource [internal]
   */
 -DWORD find_version_resource( HFILE lzfd, DWORD *reslen, DWORD *offset )
 +DECLSPEC_HIDDEN DWORD find_version_resource( HFILE lzfd, DWORD *reslen, 
 DWORD *offset )
  {

This should be in a header. It's not very useful to make it hidden if
callers are not aware of the fact.

-- 
Alexandre Julliard
julli...@winehq.org




re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

2011-05-25 Thread Dan Kegel
+static const char diffuse_paramname[] = Diffuse;
+static const char power_paramname[] = Power;
+static const char specular_paramname[] = Specular;
+static const char emissive_paramname[] = Emissive;
+static const char ambient_paramname[] = Ambient;
+static const DWORD basic_paramname_sizes = sizeof(diffuse_paramname) +
+   sizeof(power_paramname) +
+   sizeof(specular_paramname) +
+   sizeof(emissive_paramname) +
+   sizeof(ambient_paramname);
...

It might be more readable to define material_effects[] up next
to where those paramnames are defined, since it kind of
clutters the loop, and it's nicer to keep the whole table definition
together.  (Also, at least
conceptually, basic_paramname_sizes could be computed by a loop
over the table.  Then you wouldn't need those five _paramname variables,
you could just use string constants.  But if the strlen's would be
expensive, I guess that's ok.)

A comment or two about the layout of the output buffer might
be nice.

Can materials and material_ptr be const pointers?
- Dan




Tahoma Font License

2011-05-25 Thread Mark Page

I am a bit confused by the Wine tahoma license: Wine Tahoma Regular ... GNU 
Lesser General Public License 2.1 
 
It would be useful to add the font to the ClanLib SDK resources (ClanLib has a 
zlib style license) 
 
It is not clear how a font can be licensed using LGPL, since LGPL is based 
around software libraries. 
 
Ideally I feel it should have a creative commons license 
 
I believe Creative commons did not exist when that font was created
 
(I initially posted this message to forum.winehq.org, and it was suggested that 
I should post here instead)
 
Mark.
  



Re: Tahoma Font License

2011-05-25 Thread Maarten Lankhorst
Hi mark,

2011/5/25 Mark Page romb...@hotmail.co.uk:

 I am a bit confused by the Wine tahoma license: Wine Tahoma Regular ... 
 GNU Lesser General Public License 2.1

 It would be useful to add the font to the ClanLib SDK resources (ClanLib has 
 a zlib style license)

 It is not clear how a font can be licensed using LGPL, since LGPL is based 
 around software libraries.

 Ideally I feel it should have a creative commons license

 I believe Creative commons did not exist when that font was created
tahoma.sfd: Copyright: Copyright (c) 2004 Larry Snyder, Based on
Bitstream Vera Sans Copyright (c) 2003 by Bitstream, Inc. Font renamed
in accordance with former's license. Please do not contact Bitstream
Inc. for any reason regarding this font.

So just look up the original without contacting bitstream inc. ?

Cheers,
Maarten




Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

2011-05-25 Thread Adam Martinson

On 05/24/2011 03:37 AM, Huw Davies wrote:

+static IDropTarget* WrapDropTarget(IDropTarget* inner)
+{
+DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, sizeof(*This));
+
+if (This)
+{
+This-iface = (IDropTarget){DropTargetWrapper_VTbl };

This-iface.lpVtbl =DropTargetWrapper_VTbl;


-  hr = CoMarshalInterface(stream,IID_IDropTarget, unk, MSHCTX_LOCAL, NULL, 
MSHLFLAGS_TABLESTRONG);
-  IUnknown_Release(unk);
+  hr = CoMarshalInterface(stream,IID_IDropTarget, (IUnknown*)wrapper, 
MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG);

if(SUCCEEDED(hr))
{
  hr = create_map_from_stream(stream,map);
  if(SUCCEEDED(hr))
  {
-  IDropTarget_AddRef(pDropTarget);
SetPropW(hwnd, prop_oledroptarget, pDropTarget);

You probably want to set the window prop to the wrapper here.  Either
way you still need the AddRef (on the wrapper if you set that), it
gets released in RevokeDragDrop.

Huw.


The AddRef is done in WrapDropTarget(), seems like the appropriate place 
for it.  Quite right on the rest though, thanks.





Re: ole32/ole2: Don't call IDropTarget::QueryInterface() in RegisterDragDrop(). [whitespace fixed]

2011-05-25 Thread Adam Martinson

On 05/25/2011 12:36 PM, Adam Martinson wrote:

On 05/24/2011 03:37 AM, Huw Davies wrote:

+static IDropTarget* WrapDropTarget(IDropTarget* inner)
+{
+DropTargetWrapper* This = HeapAlloc(GetProcessHeap(), 0, 
sizeof(*This));

+
+if (This)
+{
+This-iface = (IDropTarget){DropTargetWrapper_VTbl };

This-iface.lpVtbl =DropTargetWrapper_VTbl;

-  hr = CoMarshalInterface(stream,IID_IDropTarget, unk, 
MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG);

-  IUnknown_Release(unk);
+  hr = CoMarshalInterface(stream,IID_IDropTarget, 
(IUnknown*)wrapper, MSHCTX_LOCAL, NULL, MSHLFLAGS_TABLESTRONG);


if(SUCCEEDED(hr))
{
  hr = create_map_from_stream(stream,map);
  if(SUCCEEDED(hr))
  {
-  IDropTarget_AddRef(pDropTarget);
SetPropW(hwnd, prop_oledroptarget, pDropTarget);

You probably want to set the window prop to the wrapper here.  Either
way you still need the AddRef (on the wrapper if you set that), it
gets released in RevokeDragDrop.

Huw.


The AddRef is done in WrapDropTarget(), seems like the appropriate 
place for it.  Quite right on the rest though, thanks.


Oops no, the window prop should be pDropTarget, not the wrapper.  The 
wrapper is only used for the marshalling.





Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

2011-05-25 Thread Dylan Smith
Hi Dan,

On Wed, May 25, 2011 at 10:31 AM, Dan Kegel d...@kegel.com wrote:
 A comment or two about the layout of the output buffer might
 be nice.


The layout doesn't group all values, param names, or defaults, so it
actually seems to be best described using loops. Does the the
following comment help summarize the layout?

/* effects buffer layout:
 *
 * D3DXEFFECTINSTANCE effects[num_materials];
 * for (effect in effects)
 * {
 * D3DXEFFECTDEFAULT defaults[effect.NumDefaults];
 * for (default in defaults)
 * {
 * *default.pParamName;
 * *default.pValue;
 * }
 * }
 */

 Can materials and material_ptr be const pointers?

ID3DXBuffer_GetBufferPointer doesn't take a const, so no for
materials, but material_ptr can be const.


The rest of the advice I took (see the attached patch).

Thanks for the patch review.
From 33f806c71c5f60240144ff30266dd7c6f3aad688 Mon Sep 17 00:00:00 2001
From: Dylan Smith dylan.ah.sm...@gmail.com
Date: Tue, 12 Apr 2011 17:35:20 -0400
Subject: [2/5] d3dx9: Generate effect instances from materials for mesh 
loading. (try 3)
Reply-To: wine-devel wine-devel@winehq.org
To: wine-patches wine-patc...@winehq.org

---
 dlls/d3dx9_36/d3dx9_36_private.h |2 +
 dlls/d3dx9_36/mesh.c |  112 --
 2 files changed, 109 insertions(+), 5 deletions(-)

diff --git a/dlls/d3dx9_36/d3dx9_36_private.h b/dlls/d3dx9_36/d3dx9_36_private.h
index f770c62..ed46402 100644
--- a/dlls/d3dx9_36/d3dx9_36_private.h
+++ b/dlls/d3dx9_36/d3dx9_36_private.h
@@ -30,6 +30,8 @@
 #include winuser.h
 #include d3dx9.h
 
+#define ARRAY_SIZE(array) (sizeof(array)/sizeof(*array))
+
 /* for internal use */
 typedef enum _FormatType {
 FORMAT_ARGB,   /* unsigned */
diff --git a/dlls/d3dx9_36/mesh.c b/dlls/d3dx9_36/mesh.c
index fa32797..631e35a 100644
--- a/dlls/d3dx9_36/mesh.c
+++ b/dlls/d3dx9_36/mesh.c
@@ -2267,6 +2267,102 @@ truncated_data_error:
 return E_FAIL;
 }
 
+static HRESULT generate_effects(ID3DXBuffer *materials, DWORD num_materials,
+ID3DXBuffer **effects)
+{
+HRESULT hr;
+D3DXEFFECTINSTANCE *effect_ptr;
+BYTE *out_ptr;
+const D3DXMATERIAL *material_ptr = ID3DXBuffer_GetBufferPointer(materials);
+static const struct {
+const char *param_name;
+DWORD name_size;
+DWORD num_bytes;
+DWORD value_offset;
+} material_effects[] = {
+#define EFFECT_TABLE_ENTRY(str, field) \
+{str, sizeof(str), sizeof(material_ptr-MatD3D.field), 
offsetof(D3DXMATERIAL, MatD3D.field)}
+EFFECT_TABLE_ENTRY(Diffuse, Diffuse),
+EFFECT_TABLE_ENTRY(Power, Power),
+EFFECT_TABLE_ENTRY(Specular, Specular),
+EFFECT_TABLE_ENTRY(Emissive, Emissive),
+EFFECT_TABLE_ENTRY(Ambient, Ambient),
+#undef EFFECT_TABLE_ENTRY
+};
+static const char texture_paramname[] = Texture0@Name;
+DWORD buffer_size;
+int i;
+
+/* effects buffer layout:
+ *
+ * D3DXEFFECTINSTANCE effects[num_materials];
+ * for (effect in effects)
+ * {
+ * D3DXEFFECTDEFAULT defaults[effect.NumDefaults];
+ * for (default in defaults)
+ * {
+ * *default.pParamName;
+ * *default.pValue;
+ * }
+ * }
+ */
+buffer_size = sizeof(D3DXEFFECTINSTANCE);
+buffer_size += sizeof(D3DXEFFECTDEFAULT) * ARRAY_SIZE(material_effects);
+for (i = 0; i  ARRAY_SIZE(material_effects); i++) {
+buffer_size += material_effects[i].name_size;
+buffer_size += material_effects[i].num_bytes;
+}
+buffer_size *= num_materials;
+for (i = 0; i  num_materials; i++) {
+if (material_ptr[i].pTextureFilename) {
+buffer_size += sizeof(D3DXEFFECTDEFAULT);
+buffer_size += sizeof(texture_paramname);
+buffer_size += strlen(material_ptr[i].pTextureFilename) + 1;
+}
+}
+
+hr = D3DXCreateBuffer(buffer_size, effects);
+if (FAILED(hr)) return hr;
+effect_ptr = ID3DXBuffer_GetBufferPointer(*effects);
+out_ptr = (BYTE*)(effect_ptr + num_materials);
+
+for (i = 0; i  num_materials; i++)
+{
+int j;
+D3DXEFFECTDEFAULT *defaults = (D3DXEFFECTDEFAULT*)out_ptr;
+
+effect_ptr-pDefaults = defaults;
+effect_ptr-NumDefaults = material_ptr-pTextureFilename ? 6 : 5;
+out_ptr = (BYTE*)(effect_ptr-pDefaults + effect_ptr-NumDefaults);
+
+for (j = 0; j  ARRAY_SIZE(material_effects); j++)
+{
+defaults-pParamName = (LPSTR)out_ptr;
+strcpy(defaults-pParamName, material_effects[j].param_name);
+defaults-pValue = defaults-pParamName + 
material_effects[j].name_size;
+defaults-Type = D3DXEDT_FLOATS;
+defaults-NumBytes = material_effects[j].num_bytes;
+memcpy(defaults-pValue, (BYTE*)material_ptr + 
material_effects[j].value_offset, defaults-NumBytes);
+out_ptr = (BYTE*)defaults-pValue + 

Re: [2/5] d3dx9: Generate effect instances from materials for mesh loading.

2011-05-25 Thread Dan Kegel
On Wed, May 25, 2011 at 6:23 PM, Dylan Smith dylan.ah.sm...@gmail.com wrote:
 The layout doesn't group all values, param names, or defaults, so it
 actually seems to be best described using loops. Does the the
 following comment help summarize the layout?

Yes.

Patch looks more readable now, thanks.  Maybe an assert
at the bottom as a sanity check that the final output
pointer is exactly at the end of the buffer would be good.
- Dan




Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

2011-05-25 Thread Stefan Dösinger
On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote:
 My first try for COM cleanup. Please donėt be too harsh :)
 If this is too large for one patch, please suggest how to split it.
 
 This is not automatic clean up so I don't really expect it to be good on
 first try.
I can't find anything wrong with it on first sight, but I don't really know how 
to manually review a patch that large with many repetitions. I guess I could 
print them on a *lot* of paper and read them at the pool or something like 
that.

I don't see an easy way to split them up either :-/ . I guess we'll have to 
trust Puk's script verification.

Just one thing: in unsafe_impl_from_IDirectDrawSurface*:

 +if (iface-lpVtbl != ddraw_surface7_vtbl) return NULL;
I don't think apps can pass in their own ddraw object implementations. I've 
never seen an app that does that or any document that suggests this is 
possible and achives something. So I'd recommend to add a FIXME() or ERR() to 
that case. An ERR because the most likely cause for such a behavior is that 
our own ddrawex.dll screws up. This is the only other place where we have a 
IDirectDrawSurface* implementation.

Or we could just remove the check and see if we ever crash because trying to 
read surface implementation fields returns odd values.


signature.asc
Description: This is a digitally signed message part.



re: Hans Leidekker : msi: Support rollback of failed uninstalls.

2011-05-25 Thread Dan Kegel
wow, msi has really come a long way!




Re: [PATCH 1/2] ddraw: COM cleanup for IDirectDrawSurface3 iface

2011-05-25 Thread Michael Stefaniuc

On 05/25/2011 08:37 PM, Stefan Dösinger wrote:

On Wednesday 25 May 2011 00:39:46 Ričardas Barkauskas wrote:

My first try for COM cleanup. Please donėt be too harsh :)
If this is too large for one patch, please suggest how to split it.

This is not automatic clean up so I don't really expect it to be good on
first try.

I can't find anything wrong with it on first sight, but I don't really know how
to manually review a patch that large with many repetitions. I guess I could
print them on a *lot* of paper and read them at the pool or something like
that.
I normally read such a patch at least 3 times, more if I find some 
issues and have to manually fix them. Well read... after hundreds of 
such patches it isn't really a conscious read anymore. More like optical 
filtering looking for odd things; that's why I *hate* inconsistent 
coding style.



I don't see an easy way to split them up either :-/ . I guess we'll have to
trust Puk's script verification.
The patch could be split up. What I do for such big and ugly patches is 
to first handle the unsafe_impl_from_IFace() part, then introduce 
impl_from_IFace() and only afterwards do the obj-lpVtl to obj-iface 
changes. Of course that really depends on the sizes of the resulting 
patches, but most of the times the unsafe_impl_from_IFace() part is a 
different patch as that isn't standard search and replace cookie cutter 
COM cleanup.



Just one thing: in unsafe_impl_from_IDirectDrawSurface*:


+if (iface-lpVtbl !=ddraw_surface7_vtbl) return NULL;

I don't think apps can pass in their own ddraw object implementations. I've
never seen an app that does that or any document that suggests this is
possible and achives something. So I'd recommend to add a FIXME() or ERR() to
that case. An ERR because the most likely cause for such a behavior is that
our own ddrawex.dll screws up. This is the only other place where we have a
IDirectDrawSurface* implementation.
Well in that case an assert() is the better option. It's done that way 
in other unsafe_impl_from_IFace like functions. And the program is 
likely to crash anyway later on when doing magic with the wrong pointer.



Or we could just remove the check and see if we ever crash because trying to
read surface implementation fields returns odd values.


bye
michael




extern char **environ and msvc

2011-05-25 Thread Stefan Dösinger
Hi,
This is one of the last remaining hacks needed to compile the d3d parts of 
Wine with Visual Studio. Our definition of **environ is conflicting with one 
already in the Windows SDK headers:

1..\..\wine\libs\wine\loader.c(54): warning C4273: '_environ': Inkonsistente 
DLL-Bindung.
1  C:\Program Files\Microsoft Visual Studio 
10.0\VC\include\stdlib.h(299): Siehe vorherige Definition von '_environ'

I'm sorry for the German, I don't know how to change this. Google finds only 
other clueless people. Either way it says something like Inconsistent DLL 
binding

The line in stdlib.h declares _environ as

_CRTIMP extern char ** _environ;/* pointer to environment table */

The compiler just writes a warning, but afterwards linking fails:

1loader.obj : error LNK2001: Nicht aufgelöstes externes Symbol __environ.

The linker complains about an unresolved external symbol __environ. Removing 
our declaration of environ fixes the warning, link error and libwine seems to 
work OK.

I don't know what the proper fix is, please advise. Maybe this code shouldn't 
be compiled at all? I don't think I need the loader code on Windows.

Stefan

From f495ac10b47d899702c1b91a8a66f042d8df71dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20D=C3=B6singer?= ste...@codeweavers.com
Date: Fri, 30 Jul 2010 06:31:40 +0200
Subject: [PATCH 1/4] libwine: msvc hack

---
 libs/wine/loader.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libs/wine/loader.c b/libs/wine/loader.c
index df0a6b5..ad0ce61 100644
--- a/libs/wine/loader.c
+++ b/libs/wine/loader.c
@@ -50,7 +50,7 @@
 #define environ (*_NSGetEnviron())
 #include CoreFoundation/CoreFoundation.h
 #include pthread.h
-#else
+#elif !defined(_MSC_VER)
 extern char **environ;
 #endif
 
-- 
1.7.3.4



signature.asc
Description: This is a digitally signed message part.



re: Hans Leidekker : msi: Support rollback of failed uninstalls.

2011-05-25 Thread Hans Leidekker
On Wed, 2011-05-25 at 18:49 +, Dan Kegel wrote:
 wow, msi has really come a long way!

Thanks. In hindsight we should have implemented rollbacks
early on, when installers still failed regularly ;-)






Re: extern char **environ and msvc

2011-05-25 Thread Frank Richter
On 25.05.2011 22:52, Stefan Dösinger wrote:
 I'm sorry for the German, I don't know how to change this. Google finds only 
 other clueless people. Either way it says something like Inconsistent DLL 
 binding

I believe the English translation is “inconsistent DLL linkage”.

 
 The line in stdlib.h declares _environ as
 
 _CRTIMP extern char ** _environ;/* pointer to environment table */
 
 The compiler just writes a warning, but afterwards linking fails:
 
 1loader.obj : error LNK2001: Nicht aufgelöstes externes Symbol __environ.
 
 The linker complains about an unresolved external symbol __environ. 
 Removing 
 our declaration of environ fixes the warning, link error and libwine seems to 
 work OK.
 
 I don't know what the proper fix is, please advise. Maybe this code shouldn't 
 be compiled at all? I don't think I need the loader code on Windows.

The underlying issue is probably the _CRTIMP - that's most likely
_declspec(dllimport) in MSVC. This matters, as DLL-imported and “plain”
extern variables are handled differently on Windows.
IIRC DLL export/import of variables is indirect - when DLL-importing a
variable you really import a _pointer_ to the variable. So accessing the
variable dereferences some pointer. “extern” variables, otoh, are
accessed directly (resolving the actual variable location is handled at
link-time).

-f.r.



signature.asc
Description: OpenPGP digital signature



Re: extern char **environ and msvc

2011-05-25 Thread Stefan Dösinger
On Wednesday 25 May 2011 23:04:02 Frank Richter wrote:
 The underlying issue is probably the _CRTIMP - that's most likely
 _declspec(dllimport) in MSVC.
I suspected something like this, but msvc didn't want to tell me what _CRTIMP 
was. Either way it doesn't get me much closer to a fix.



signature.asc
Description: This is a digitally signed message part.



Re: [2/2] user32: Add message test for hotkeys.

2011-05-25 Thread Marvin
Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
http://testbot.winehq.org/JobDetails.pl?Key=11184

Your paranoid android.


=== W7PROX64 (64 bit msg) ===
Timeout




Re: [1/2] user32: Add test for RegisterHotKey/UnregisterHotKey.

2011-05-25 Thread Marvin
Hi,

While running your changed tests on Windows, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?
Full results can be found at
http://testbot.winehq.org/JobDetails.pl?Key=11183

Your paranoid android.


=== W7PROX64 (64 bit msg) ===
Timeout




Re: Tahoma Font License

2011-05-25 Thread Scott Ritchie
On 05/25/2011 09:27 AM, Maarten Lankhorst wrote:
 Hi mark,
 
 2011/5/25 Mark Page romb...@hotmail.co.uk:

 I am a bit confused by the Wine tahoma license: Wine Tahoma Regular ... 
 GNU Lesser General Public License 2.1

 It would be useful to add the font to the ClanLib SDK resources (ClanLib has 
 a zlib style license)

 It is not clear how a font can be licensed using LGPL, since LGPL is based 
 around software libraries.

 Ideally I feel it should have a creative commons license

 I believe Creative commons did not exist when that font was created
 tahoma.sfd: Copyright: Copyright (c) 2004 Larry Snyder, Based on
 Bitstream Vera Sans Copyright (c) 2003 by Bitstream, Inc. Font renamed
 in accordance with former's license. Please do not contact Bitstream
 Inc. for any reason regarding this font.
 
 So just look up the original without contacting bitstream inc. ?
 

The topic of relicensing wine fonts has come up before.  Usually the
discussion ends with our fonts aren't as good as you think, and you
probably don't want them so don't worry about licensing.

I'm not so sure it's true anymore.  Symbol replacement, for instance,
can actually be useful to show special characters on web pages that
would otherwise come up as question marks.

-Scott Ritchie