Re: Add Windows Vista option to winecfg
Kovács András wrote: This patch is needed, because DirectX 10 is Vindows Vista only feature. Nothing wrong with adding Vista to the list but how is this needed? You don't want to disallow usage of dx10 unless Vista is selected, do you? Felix
Re: Patches / a proposal for the mystic DIB engine
Alexandre Julliard wrote: 2. Export LockDIBSection/Unlock to gdi32. Adding more exports is not nice but there really is no way around that, right? No, LockDIBSection is a driver internal detail, gdi32 has no business knowing about this. In my code the call to LockDIBSection serves two purposes: to lock the DIB section (duh) and to query whether we are in AppMod or GdiMod. The former could be done by triggering an exception and letting the driver catch it that but I'd think that's rather expensive. The latter can't be done at all if this is an "driver internal detail" so we'd have to always use the DIBDRV implementation when it is there. This would most likely mean some pretty serious performance regressions until we implemented all used functions (in which case we could say the export is only temporary). Since we'll at some point probably have all functions implemented the DIB code in the driver will be dead code and it would have to be removed anyway. So I don't see the point of trying to keep a hack clean. Any another issue: There are cases where we really want to have the GDI operation be done server-side, regardless of whether we have a client-side implementation or not (http://www.winehq.com/pipermail/wine-devel/2007-February/053993.html - I'd agree with that). So I really think we need that export. 3. Move dc->funcs to dc->physDev->funcs. Many changes but mostly mechanical work. Rationale: This really belongs there. And I need it. :) No it doesn't, physDev is an opaque structure as far as gdi32 is concerned. Data needed by gdi32 belongs in the DC structure. Well, yes, it's an opaque structure. With the function pointers added to physDev it would no longer be an opaque structure. So? We'd add a pointer to physDev pointing to an opaque structure of course. My point is that the addresses of the driver functions are not "data needed by gdi32" in the strictest sense. Those addresses are defined by the driver and the functions live in the driver, so I think it makes sense if the driver would initialize the table, not gdi32. Doing this by GetProcAddress() in gdi32/driver.c is not a very clean design. Really. You certainly don't want to store the full function table in the BITMAPOBJ, it will be the same for all bitmaps. All you need is one Will it be the same? I'm not sure if there are situations where an application has multiple drivers providing memory DCs loaded. function table for the DIB driver and one for the normal graphics driver. Forwarding to the graphics driver can be done privately in the DIB driver, gdi32 doesn't need to know about it. And you probably Yes, it's possible. But we'd probably have to write a wrapper for every GDI operation. Wouldn't be hard - but it's a lot of code which can be easily avoided by just 10 lines outside of dibdrv (in gdi32/bitmap.c). I don't really see the point. want a separate physDev pointer for it, you'll need to maintain state for DIBs too. Well, we have a couple of DIB-dependent members in BITMAPOBJ - I'd put a pointer to this state there. I admit this isn't really clean though. Felix
Re: Patches / a proposal for the mystic DIB engine
llPath */
EMFDRV_StrokePath, /* pStrokePath */
NULL,/* pSwapBuffers */
+NULL,/* pUnlockDIBSection */
NULL,/* pUnrealizePalette */
EMFDRV_WidenPath /* pWidenPath */
};
diff --git a/dlls/gdi32/gdi_private.h b/dlls/gdi32/gdi_private.h
index e43ac30..6d06e02 100644
--- a/dlls/gdi32/gdi_private.h
+++ b/dlls/gdi32/gdi_private.h
@@ -97,6 +97,9 @@ typedef struct tagGDIOBJHDR
typedef struct { int opaque; } *PHYSDEV; /* PHYSDEV is an opaque pointer */
+/* For pLockDIBSection() */
+enum { DIB_Status_None, DIB_Status_InSync, DIB_Status_GdiMod, DIB_Status_AppMod };
+
typedef struct tagDC_FUNCS
{
INT (*pAbortDoc)(PHYSDEV);
@@ -150,6 +153,7 @@ typedef struct tagDC_FUNCS
INT (*pIntersectClipRect)(PHYSDEV,INT,INT,INT,INT);
BOOL (*pInvertRgn)(PHYSDEV,HRGN);
BOOL (*pLineTo)(PHYSDEV,INT,INT);
+HBITMAP (*pLockDIBSection)(PHYSDEV,BOOL,BOOL);
BOOL (*pModifyWorldTransform)(PHYSDEV,const XFORM*,INT);
BOOL (*pMoveTo)(PHYSDEV,INT,INT);
INT (*pOffsetClipRgn)(PHYSDEV,INT,INT);
@@ -219,6 +223,7 @@ typedef struct tagDC_FUNCS
BOOL (*pStrokeAndFillPath)(PHYSDEV);
BOOL (*pStrokePath)(PHYSDEV);
BOOL (*pSwapBuffers)(PHYSDEV);
+VOID (*pUnlockDIBSection)(PHYSDEV,BOOL);
BOOL (*pUnrealizePalette)(HPALETTE);
BOOL (*pWidenPath)(PHYSDEV);
diff --git a/dlls/gdi32/mfdrv/init.c b/dlls/gdi32/mfdrv/init.c
index e939ce1..88e102d 100644
--- a/dlls/gdi32/mfdrv/init.c
+++ b/dlls/gdi32/mfdrv/init.c
@@ -84,6 +84,7 @@ static const DC_FUNCTIONS MFDRV_Funcs =
MFDRV_IntersectClipRect, /* pIntersectClipRect */
MFDRV_InvertRgn, /* pInvertRgn */
MFDRV_LineTo,/* pLineTo */
+NULL,/* pLockDIBSection */
NULL,/* pModifyWorldTransform */
MFDRV_MoveTo,/* pMoveTo */
MFDRV_OffsetClipRgn, /* pOffsetClipRgn */
@@ -151,6 +152,7 @@ static const DC_FUNCTIONS MFDRV_Funcs =
MFDRV_StrokeAndFillPath, /* pStrokeAndFillPath */
MFDRV_StrokePath,/* pStrokePath */
NULL,/* pSwapBuffers */
+NULL,/* pUnlockDIBSection */
NULL,/* pUnrealizePalette */
MFDRV_WidenPath /* pWidenPath */
};
diff --git a/dlls/winex11.drv/winex11.drv.spec b/dlls/winex11.drv/winex11.drv.spec
index 6a6c173..abb9279 100644
--- a/dlls/winex11.drv/winex11.drv.spec
+++ b/dlls/winex11.drv/winex11.drv.spec
@@ -29,6 +29,7 @@
@ cdecl GetTextExtentExPoint(ptr ptr long long ptr ptr ptr) X11DRV_GetTextExtentExPoint
@ cdecl GetTextMetrics(ptr ptr) X11DRV_GetTextMetrics
@ cdecl LineTo(ptr long long) X11DRV_LineTo
+@ cdecl LockDIBSection(ptr long long) X11DRV_LockDIBSection
@ cdecl PaintRgn(ptr long) X11DRV_PaintRgn
@ cdecl PatBlt(ptr long long long long long) X11DRV_PatBlt
@ cdecl Pie(ptr long long long long long long long long) X11DRV_Pie
@@ -59,6 +60,7 @@
@ cdecl SetTextColor(ptr long) X11DRV_SetTextColor
@ cdecl StretchBlt(ptr long long long long ptr long long long long long) X11DRV_StretchBlt
@ cdecl SwapBuffers(ptr) X11DRV_SwapBuffers
+@ cdecl UnlockDIBSection(ptr long) X11DRV_UnlockDIBSection
@ cdecl UnrealizePalette(long) X11DRV_UnrealizePalette
# USER driver
diff --git a/dlls/gdi32/Makefile.in b/dlls/gdi32/Makefile.in
index e3b2d67..279df01 100644
--- a/dlls/gdi32/Makefile.in
+++ b/dlls/gdi32/Makefile.in
@@ -22,6 +22,7 @@ C_SRCS = \
clipping.c \
dc.c \
dib.c \
+ dibdrv.c \
driver.c \
enhmetafile.c \
enhmfdrv/bitblt.c \
diff --git a/dlls/gdi32/bitmap.c b/dlls/gdi32/bitmap.c
index d16e70f..d59906b 100644
--- a/dlls/gdi32/bitmap.c
+++ b/dlls/gdi32/bitmap.c
@@ -586,6 +586,18 @@ static HGDIOBJ BITMAP_SelectObject( HGDIOBJ handle, void *obj, HDC hdc )
if (handle)
{
+BITMAPOBJ *old = GDI_GetObjPtr( dc->hBitmap, BITMAP_MAGIC );
+if (old)
+{
+/* When switching from non-DIB to DIB or vice versa adjust
+ * DC function pointers. */
+if (!old->dib && bitmap->dib)
+DIBDRV_Install( dc, bitmap );
+else if (old->dib && !bitmap->dib)
+DIBDRV_Remove( dc, old );
+}
+GDI_ReleaseObj( old );
+
dc->hBitmap = handle;
dc->flags &= ~DC_DIRTY;
SetRectRgn( dc->hVisRgn, 0, 0, bitmap->bitmap.bmWidth, bitmap->bitmap.bmHeight);
diff --git a/dlls/gdi32/dibdrv.c b/dlls/gdi32/dibdrv.c
new file mode 100644
index 000..e4a216e
--- /dev/null
+++ b/dlls/gdi32/dibdrv.c
@@ -0,0 +1,190 @@
+/*
+ * DIB driver
+ *
+ * Copyright 2007 Felix Nawothnig
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Les
Patches / a proposal for the mystic DIB engine
Okay, I've spent the last days looking into this matter and I'd like to
suggest a way to get it started. So. This is the plan:
1. In winex11.drv:
-INT X11DRV_LockDIBSection(X11DRV_PDEVICE *physDev, INT req, BOOL lossy)
+HBITMAP X11DRV_LockDIBSection(X11DRV_PDEVICE *physDev, INT req, BOOL force)
Lossy isn't used anywhere for LockDIBSection anyway. Force means that
the function will only lock if the DIB is already in AppMod. The
returned bitmap is ofcourse physDev->bitmap->hbitmap.
Rationale: If you lock the DIB to write on it it makes sense that the
function actually provides you with that DIB.
2. Export LockDIBSection/Unlock to gdi32.
Adding more exports is not nice but there really is no way around
that, right?
3. Move dc->funcs to dc->physDev->funcs. Many changes but mostly
mechanical work.
Rationale: This really belongs there. And I need it. :)
4. Now we write dibdrv.c for now just containing DIBDRV_Install
and DIBDRV_Remove. That function will go through the physDev->funcs
list and overwrite each function pointer which is actually
implemented with DIBDRV_PutPixel(), whatever.
DIBDRV_Install/DIBDRV_Remove will be called from
BITMAP_SelectObject() when we switch from non-DIB to DIB vice versa.
Note that we can't use DRIVER_load_driver here because of the
wanted "forward to original driver when not implemented".
For this we will need to extend the "for DIB objects" part in
BITMAPOBJ by
const DC_FUNCTIONS *orig_funcs;
DC_FUNCTIONSlocal_funcs;
where orig_funcs to the old physDev->funcs and the new physDev->funcs
points to &bmp->local_funcs.
5. In dibdrv.c we get us:
enum {
DIBDRV_MIXED, /* Choose driver depending on current AppMod */
DIBDRV_ALWAYS, /* Always use DIBDRV (unless function not
implemented) */
DIBDRV_NEVER /* Always use DC driver */
} DIB_Mode = DIBDRV_MIXED;
and DIB_Lock(PHYSDEV physDev) / DIB_Unlock(PHYSDEV).
Now, here comes the reason we need the HBITMAP from
LockDIBSection() and funcs inside PHYSDEV:
Since we don't have the DC here we have no way of
calling LockDIBSection(), forwarding to the original driver
if NEVER/MIXED or writing to the actual DIB in case of
MIXED/ALWAYS.
6. From this point we can start implementing one function at a time,
touching nothing but dibdrv.c.
So far so good. I did all those steps (except 3., I hacked around that
in my local tree) in a clean way and it works nice (for 6. I implemented
SetPixel() and tried that with a test-app).
I attached patches for the steps 1 2 and 4 so you can see where this is
going.
Comments?
Felix
Re: notepad: Fix the wrong '&' operator use.
Byeong-Sik Jeon wrote: FormatMessage( FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM, NULL, error, 0, -(LPTSTR) &lpMsgBuf, 0, NULL); +lpMsgBuf, 0, NULL); This patch is wrong. When FORMAT_MESSAGE_ALLOCATE_BUFFER is used FormatMessage() takes a pointer to a pointer.
Re: [1/2] wined3d: Downgrade some ERRs to FIXMEs
Stefan Dösinger wrote: Some of them should technically be ERRs because we tell the application that we do not support those features, so they should never be used. Application bugs shouldn't trigger ERRs, Wine bugs should. Felix.
Re: Setting debug channels in 20040914 version
Bill Medland wrote: Can someone remind me how we used to set the debug channels back in late 2004? wine --debugmsg +relay ... Or maybe it was -debugmsg? Felix
Re: Is Wine a platform for Codeweaver to make money?! Please help me understand.
Sasan Iman wrote: I know that if you fiddle around with the stock release long enough you can get MS Office working on Wine as I have on a number of occasions, but shouldn't making this work for everyone be the highest priority for getting Wine to be adopted more widely?! Or is Wine more targeted to gamers which is mostly what I read about on WineHQ? My guess: Wine is not a product so it's not targeted to anyone. It's a community project developed by volunteers - and the primary priority of those volunteers is to get Wine to run the applications they want to use. Since there are many nice free & libre cross-platform alternatives to the MS office suite there is just not much incentive to get it to run or keep it running. Games OTOH... Felix
Re: gdi32: Add failing metrics test for negative font widths
Felix Nawothnig wrote: --- dlls/gdi32/tests/font.c | 37 + 1 files changed, 37 insertions(+), 0 deletions(-) Anything wrong with this patch?
Re: WineD3D: Make CreateCubeTexture fail when not supported
Duane Clark wrote: On a Matrox G550, Win2k, crosscompiled on Linux with MingW: I:\dlls\d3d8\tests>d3d8_crosstest texture texture.c:122:texture caps: 0x41c7 texture.c:110:pool=0 hr=0x8876086c texture.c:110:pool=1 hr=0x8876086c texture.c:110:pool=2 hr=0x8876086c texture.c:110:pool=3 hr=0x texture: 57 tests executed (0 marked as todo, 0 failures), 0 skipped. Thanks. Guess the patch is okay to commit then.
Re: WineD3D: Make CreateCubeTexture fail when not supported
Luke Bratch wrote: Somewhere I have an ancient Matrix MGA G200 AGP card... If this would be of any use, I'll dig it out and run any tests in Wine/Windows that you'd like? Sure, if you're willing to do that just because I'm lazy. :-) I was just hoping someone might be running a box with an MGA and Windows installed as I'd have to reconnect my CDROM drive, steal a HDD from my brother and install Windows first. Felix
WineD3D: Make CreateCubeTexture fail when not supported
Not tested under Windows (does _anyone_ besides me have a Matrox? :) -
would be nice if someone with either an MGA or an really ancient GPU
could run the test on windows (if the pool=0 trace has an hr!=0 you got
one of those ancient cards :). CCing to wine-devel for that reason.
---
dlls/d3d8/tests/texture.c | 27 +++
dlls/wined3d/device.c |5 +
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/dlls/d3d8/tests/texture.c b/dlls/d3d8/tests/texture.c
index 3e0b805..ea95287 100644
--- a/dlls/d3d8/tests/texture.c
+++ b/dlls/d3d8/tests/texture.c
@@ -101,6 +101,32 @@ static void test_texture_stage_states(IDirect3DDevice8 *device_ptr, int num_stag
}
}
+static void test_cube_texture_from_pool(IDirect3DDevice8 *device_ptr, DWORD caps, D3DPOOL pool, BOOL need_cap)
+{
+IDirect3DCubeTexture8 *texture_ptr = NULL;
+HRESULT hr;
+
+hr = IDirect3DDevice8_CreateCubeTexture(device_ptr, 512, 1, 0, D3DFMT_X8R8G8B8, pool, &texture_ptr);
+trace("pool=%d hr=0x%.8x\n", pool, hr);
+
+if((caps & D3DPTEXTURECAPS_CUBEMAP) || !need_cap)
+ok(SUCCEEDED(hr), "hr=0x%.8x\n", hr);
+else
+ok(hr == D3DERR_INVALIDCALL, "hr=0x%.8x\n", hr);
+
+if(texture_ptr) IDirect3DCubeTexture8_Release(texture_ptr);
+}
+
+static void test_cube_textures(IDirect3DDevice8 *device_ptr, DWORD caps)
+{
+trace("texture caps: 0x%.8x\n", caps);
+
+test_cube_texture_from_pool(device_ptr, caps, D3DPOOL_DEFAULT, TRUE);
+test_cube_texture_from_pool(device_ptr, caps, D3DPOOL_MANAGED, TRUE);
+test_cube_texture_from_pool(device_ptr, caps, D3DPOOL_SYSTEMMEM, TRUE);
+test_cube_texture_from_pool(device_ptr, caps, D3DPOOL_SCRATCH, FALSE);
+}
+
START_TEST(texture)
{
D3DCAPS8 caps;
@@ -120,4 +146,5 @@ START_TEST(texture)
IDirect3DDevice8_GetDeviceCaps(device_ptr, &caps);
test_texture_stage_states(device_ptr, caps.MaxTextureBlendStages);
+test_cube_textures(device_ptr, caps.TextureCaps);
}
diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c
index 7863714..ea09780 100644
--- a/dlls/wined3d/device.c
+++ b/dlls/wined3d/device.c
@@ -999,6 +999,11 @@ static HRESULT WINAPI IWineD3DDeviceImpl_CreateCubeTexture(IWineD3DDevice *iface
return WINED3DERR_INVALIDCALL;
}
+if (!GL_SUPPORT(ARB_TEXTURE_CUBE_MAP) && Pool != WINED3DPOOL_SCRATCH) {
+WARN("(%p) : Tried to create not supported cube texture\n", This);
+return WINED3DERR_INVALIDCALL;
+}
+
D3DCREATERESOURCEOBJECTINSTANCE(object, CubeTexture, WINED3DRTYPE_CUBETEXTURE, 0);
D3DINITIALIZEBASETEXTURE(object->baseTexture);
Re: x11drv: XDND: Fix file drop to properly support file:/// URIs(debugging problem)
Maciej Katafiasz wrote:
+if(GetLastError())
+{
+ TRACE("Can't convert to WCHAR: %d\n", GetLastError());
+ goto clean_wfn;
+}
This is not an appropriate way of testing for an API failure.
What is inappropriate and how should I fix it?
Most APIs don't change GLE on success. Some do but most often the
behaviour is undocumented or changed between 9x/NT. You should check the
return value which usually is either 0 or -1 (depends on the API) on
failure.
Felix
Re: Supreme Commander Up and running!
Stephan Rose wrote: Figure, that'd be as good as a start as any as something for me to implement. Easy to test toojust before I dive into it, anyone potentially already working on it (or is there a good way to check)? Figured I'd ask first before 2 people work on the same thing. Someone sent a patch to implement that function one or two weeks ago. Didn't get commited though. Felix
Re: [2/3] comctl32: Never draw item text with OPAQUE.
Dimi Paun wrote: Depends. Right now this code does nothing (and I was going to remove it anyway) - the only place where clrTextBk is read from is in prepaint_setup() - but prepaint_setup() is not called for subitems (and the CLR_NONE override is only done for subitems). Well, but that's the point: now that prepaint_setup() is always called (I agree with that change), this would do what we want if only we said the CLR_NONE _before_ the call to prepaint_setup() where it's being acted on. Doesn't work because prepaint_setup() overrides the color again. Anyway, I'm revising my patches... Felix
Re: [2/3] comctl32: Never draw item text with OPAQUE.
(CCing wine-devel again) Dimi Paun wrote: Problem is that DrawText() for the subitems will also use the background color - which is wrong. Why is it wrong? Because the "background color" isn't the background color of the list. And it's definitly not the text bkclr in gdi-context either - I hacked together an app to prove that: http://flexo.popuserv.org/test.c Now it seems getting rid of the OPAQUE completly is wrong, but for different reasons. It seems to me that this is a bit of a problem: 3783 if (nSubItem == 0 || (cdmode & CDRF_NOTIFYITEMDRAW)) 3784 prepaint_setup(infoPtr, hdc, &nmlvcd); 3785 3786 /* in full row select, subitems, will just use main item's colors */ 3787 if (nSubItem && uView == LVS_REPORT && (infoPtr->dwLvExStyle & LVS_EX_FULLROWSELECT)) 3788 nmlvcd.clrTextBk = CLR_NONE; I guess the call to prepaint_setup should come _after_ the if with FULLROWSELECT, no? Depends. Right now this code does nothing (and I was going to remove it anyway) - the only place where clrTextBk is read from is in prepaint_setup() - but prepaint_setup() is not called for subitems (and the CLR_NONE override is only done for subitems). Also, I can't remember, but there was a reason why prepain_setup was called just for the main item, why did you changed that? Because subitems can use different colors (see test app) and prepaint_setup() is where the colors are set up... Note that I don't change the notifications being passed or anything of their context (with the exception of the GDI context in the postpaint notification - but the GDI context should be saved & restored in between anyway, see the failing test I commited a few days ago) so I can't imagine how this could break anything. Felix
Re: [2/3] comctl32: Never draw item text with OPAQUE.
Felix Nawothnig wrote: Well - yes. But other controls do this too (statusbar for example) and considering all the stuff we do during refresh this is hardly a bottleneck. And even if it is we should optimize this codepath in gdi32 instead (by caching solid brushs for example). ...looking at the statusbar code again I think using GetSysColorBrush() for the three default cases would be even better. If an app overrides the colors using CD performance will suffer slightly but that's Microsoft's fault by requesting a COLORREF and not a brush for each item. Felix
Re: [2/3] comctl32: Never draw item text with OPAQUE.
Dimi Paun wrote: But this ends up creating and deleting the brush for each of the cells Well - yes. But other controls do this too (statusbar for example) and considering all the stuff we do during refresh this is hardly a bottleneck. And even if it is we should optimize this codepath in gdi32 instead (by caching solid brushs for example). we draw... I'm not sure I understand why using OPAQUE here doesn't work. Problem is that DrawText() for the subitems will also use the background color - which is wrong. To fix this while still using OPAQUE we'd need to do something like: SetBkMode(OPAQUE); TextOutW(...); SetBkMode(TRANSPARENT); But abusing TextOutW() like that just seems wrong to me. Using DrawText() to draw the background won't work either due to LVS_EX_FULLROWSELECT (well, I guess we *could* do it by DrawText()ing the first item over the full row and the subitems on top of it but I'd prefer to _simplify_ the code, not add more hacks). Felix
Defining COM interfaces without IDL
Hi. The PSDK headers sometimes use IDL to define an interface, sometimes it's done by hand (or rather using macros). I don't know if there is any reason for that (someone in the channel said it's just sloppiness by MS) - so, are there any resulting differences regarding source compatibility or can we just use IDL even if MS didn't? Felix
Re: comctl32: register all the controls in DllMain (fixes bug #7641)
Mikołaj Zalewski wrote: comctl32 version 5.82 (at least the one that comes with a fully patched Windows XP) and 6.0 register all the controls in DllMain. InitCommonControlsEx is just a dummy function. Could it be that 6.0 only registers those controls listed using window class dependencies in the manifest file? If what you say about 5.82 is true your patch is correct though ofcourse. Felix
Re: appdb needs to allow untested applications sometimes
Chris Morgan wrote: What would the purpose of having untested applications in the appdb be? Users would search for the app and find that it shows up but then find that it doens't work. At which point the application would qualify as been tested, no? Felix
Re: kernel32: MultiByteToWideChar: MB_USEGLYPHCHARS fix
Andrew O.Shadoura wrote:
+#if 0
static int once;
+#endif
if (!src || (!dst && dstlen))
{
@@ -1774,11 +1783,13 @@ INT WINAPI MultiByteToWideChar( UINT pag
if (srclen < 0) srclen = strlen(src) + 1;
+#if 0
if (!once && (flags & MB_USEGLYPHCHARS))
{
once = 1;
FIXME("MB_USEGLYPHCHARS not supported\n");
}
+#endif
If you implemented MB_USEGLYPHCHARS just remove the FIXME. Don't submit
dead code - git will remember all revisions...
Felix
Re: quartz: Check allocation failure and clear memory inDSound Renderer
Dmitry Timoshkov wrote: However, note that NULL is not always all binary zero in memory. :) I don't believe it's true since NULL is defined as (void *)0. Actually it may aswell be just 0 in C. Just in C++ it's defined to be (void *)0. But even with just 0 an assignment/compare/whatever will get you an implicit typecast which makes the compiler generate any necessary conversion. See http://c-faq.com/null/ Felix
Re: quartz: Check allocation failure and clear memory in DSound Renderer
Detlef Riekenberg wrote: @@ -325,9 +328,15 @@ HRESULT DSoundRender_create(IUnknown * pUnkOuter, LPVOID * ppv) pDSoundRender->pClock = NULL; pDSoundRender->init = FALSE; pDSoundRender->started = FALSE; ... there is no need to clear it again. However, note that NULL is not always all binary zero in memory. :)
Re: user32: avoid NULL pointer access in DefWindowProcA WM_NCCREATE
Jan Zerebecki wrote: cs is never NULL at that point. Where should it be checked for NULL, then? Or what does it prevent to be NULL? cs == lParam. And this code is inside if (lParam != NULL).
Re: user32: avoid NULL pointer access in DefWindowProcA WM_NCCREATE
Jan Zerebecki wrote: CREATESTRUCTA *cs = (CREATESTRUCTA *)lParam; /* check for string, as static icons, bitmaps (SS_ICON, SS_BITMAP) * may have child window IDs instead of window name */ -if (HIWORD(cs->lpszName)) +if (cs && HIWORD(cs->lpszName)) cs is never NULL at that point. Felix
Re: Work legalities
Nathan Williams wrote: What do I need from my employer to clear me to work on wine? Is something verbal ok, or should I get it in writing? The FSF says: http://www.gnu.org/licenses/gpl.html | You should also get your employer (if you work as a programmer) or | your school, if any, to sign a "copyright disclaimer" for the program, | if necessary. Here is a sample; alter the names: | | Yoyodyne, Inc., hereby disclaims all copyright interest in the program | `Gnomovision' (which makes passes at compilers) written by James | Hacker. | | signature of Ty Coon, 1 April 1989 | Ty Coon, President of Vice Felix
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
Erich Hoover wrote: I see your point. However, since the function you are implementing is in kernel32 anyway you could abstract it away and make both functions (CopyFile and ReplaceFile) call some internal function. That way you would get rid of the locking completly which is argueably somewhat ugly. On closer inspection, CreateFile actually seems to take care of this with the "security attributes" and "template" parameters. It does not look like the security attributes are implemented yet (or the call to get the security attributes for a file) but since CreateFile already handles this it seems unnecessary to add a new break-out function. You will likely find the attached more to your liking. Doesn't matter what I like, in the end it's up to Alexandre. But yes, looks much better to me (although I'd still say it makes sense to abstract away the actually copying in a subroutine). Felix
Re: Forum proposal
(Score:-1, Flaimbait) Wojciech 'arab' Arabczyk wrote: Well - one could setup a forum<->mailing list gateway. Not easily doable due to the nature of SMTP. Also a forum would be completly useless for anything but wine-users... But let's be honest here: Most people who use webforums are too stupid or lazy to set up or use an SMTP/IMAP/NNTP client. And don't get me even started about usability of most forums, especially phpBB and the like. No proper threading but cool animated avatars, ranks and post counts... Yes, there is RForum (and I believe someone actually wrote an SMTP gateway for it) and others but I doubt it would be worth the hassle. And it's not like we are short of bug reports - do we really want to deal with people who will post on a forum but refuse or are unable to sign on at an ML? However - a new web-interface for the ml-archives would be nice (with integrated search, no silly month-splits and... well... a somewhat prettier design IMHO. Gmane is there but I usually end up using google on the winehq archives though... Felix
Re: Wine's Wiki Defaced
Francois Gouget wrote: Someone has defaced Wine's Wiki frontpage. Also I have either forgotten my password or the user accounts don't work anymore. Could someone look into this? Reverted it (no idea about the accounts, I was still logged in).
Re: Using ex-user32 controls from native comctl32 6
Frank Richter wrote: So, any idea how Windows makes comctl32 register the user classes? I think it just re-registered the classes, but I'm not sure. A trace with class registrations (ie +class) might give some hint at what native does. Okay, I was stupid. I removed registration of all the user32 classes, only some were copied over to comctl32. Anyway, it works "fine" know. Well. Actually it doesn't even initialize without some stubs but it runs. I have a native edit control in wine for the first time since 9x times eh? :) Felix
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
Erich Hoover wrote: The "right" way would probably to do the copying yourself by read/write.. but I dunno. Except that it would ignore the permissions issues that have already been coded into the copy routines (and any updates that may eventually No, CreateFile (and friends) does the permissions checks (which you would still have to call). That was worded poorly, Copy/Move already handle copying file attributes and I imagine would eventually implement copying the access control list information. Implementing ReplaceFile as calls to either Copy or Move takes these issues into account. I see your point. However, since the function you are implementing is in kernel32 anyway you could abstract it away and make both functions (CopyFile and ReplaceFile) call some internal function. That way you would get rid of the locking completly which is argueably somewhat ugly. Felix
Using ex-user32 controls from native comctl32 6
(CC-ing to Frank Richter because I hope he knows the answer :) Hi. I've been trying to make wine use the ex-user32 controls (listbox, scrollbox, etc.) in comctl32 where Microsoft copied them to in version 6 to get message traces of those controls (I know I could use msg spy tools on Windows but comparing two Wine spy logs is way more straight forward, especially since those tools don't get you stuff like +win). You need a manifest file on Windows to get application to use comctl6 but we don't use that since we've chosen to theme the user controls via subclassing. I've tried to just remove registration of the classes from our user32 and use native comctl32 6 (which otherwise works nice) but no success. So, any idea how Windows makes comctl32 register the user classes? Felix
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
(CC-ing wine-devel again)
Erich Hoover wrote:
The "right" way would probably to do the copying yourself by
read/write.. but I dunno.
Except that it would ignore the permissions issues that have already
been coded into the copy routines (and any updates that may eventually
No, CreateFile (and friends) does the permissions checks (which you
would still have to call).
be made to these routines). Also, it seems to me that MSDN is
describing the list of function calls that ReplaceFile actually makes.
No it doesn't?
Your other objections were right. Your last version follows for wine-devel.
/**
* ReplaceFileW (KERNEL32.@)
* ReplaceFile(KERNEL32.@)
*/
BOOL WINAPI ReplaceFileW(LPCWSTR lpReplacedFileName,LPCWSTR
lpReplacementFileName,
LPCWSTR lpBackupFileName, DWORD dwReplaceFlags,
LPVOID lpExclude, LPVOID lpReserved)
{
HANDLE hReplaced, hReplacement;
BOOL skipBackup = FALSE;
if(dwReplaceFlags)
FIXME("Ignoring flags %x\n", dwReplaceFlags);
/* First two arguments are mandatory */
if(!lpReplacedFileName || !lpReplacementFileName)
{
SetLastError( ERROR_INVALID_PARAMETER );
return FALSE;
}
/*
* Lock the "replacement" file, fail if it does not exist
*/
if((hReplacement = CreateFileW(lpReplacementFileName, GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL, OPEN_EXISTING, 0, 0)) == INVALID_HANDLE_VALUE)
{
return FALSE;
}
if(!LockFile( hReplacement, 0, 0, 0, 0 ))
{
CloseHandle( hReplacement );
return FALSE;
}
/*
* Lock the "replaced" file while we're working.
*/
if((hReplaced = CreateFileW(lpReplacedFileName, GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE,
NULL, OPEN_EXISTING, 0, 0)) == INVALID_HANDLE_VALUE)
{
/* If "replaced" does not exist then create it for the lock, but skip
backup */
if((hReplaced = CreateFileW(lpReplacedFileName, GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE,
NULL, CREATE_ALWAYS, 0, 0)) == INVALID_HANDLE_VALUE)
{
UnlockFile( hReplacement, 0, 0, 0, 0 );
CloseHandle( hReplacement );
return FALSE;
}
skipBackup = TRUE;
}
if(!LockFile( hReplaced, 0, 0, 0, 0 ))
{
UnlockFile( hReplacement, 0, 0, 0, 0 );
CloseHandle( hReplacement );
CloseHandle( hReplaced );
return FALSE;
}
/*
* If the user wants a backup then that needs to be performed first
*/
if( lpBackupFileName && !skipBackup )
{
/* If an existing backup exists then copy over it */
if(!CopyFileW( lpReplacedFileName, lpBackupFileName, FALSE ))
goto replace_fail;
}
/*
* Now that the backup has been performed (if requested), copy the
replacement
* into place
*/
if(!CopyFileW( lpReplacementFileName, lpReplacedFileName, FALSE ))
{
/* Assume that all access denied errors are a write problem. */
if(GetLastError() == ERROR_ACCESS_DENIED)
SetLastError( ERROR_UNABLE_TO_REMOVE_REPLACED );
else
SetLastError( ERROR_UNABLE_TO_MOVE_REPLACEMENT );
goto replace_fail;
}
/* Delete the replacement file */
if(!DeleteFileW( lpReplacementFileName ))
return FALSE;
/* Unlock the handles */
UnlockFile( hReplacement, 0, 0, 0, 0 );
CloseHandle( hReplacement );
UnlockFile( hReplaced, 0, 0, 0, 0 );
CloseHandle( hReplaced );
/* All done, file replacement successful */
return TRUE;
replace_fail:
UnlockFile( hReplacement, 0, 0, 0, 0 );
CloseHandle( hReplacement );
UnlockFile( hReplaced, 0, 0, 0, 0 );
CloseHandle( hReplaced );
return FALSE;
}
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
Erich Hoover wrote: Is the attached more like what you're looking for? I did some investigation and... actually I'm looking for an equivalent to the linux splice syscall. Win32 is the most braindead API ever. Duh. The "right" way would probably to do the copying yourself by read/write.. but I dunno. Your code looks solid at the first glance. Three things: - I'd remove the FILE_SHARE_DELETE and FILE_SHARE_WRITE. - Use MoveFile instead of CopyFile/DeleteFile at the end? - Are you sure you're supposed to set ERROR_INVALID_PARAMETER no matter what bad happens? If the function which failed already set LE you should probably keep it... Felix
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
Erich Hoover wrote:
I assume you're referring to the file existence check and file delete,
followed by the actual copy and move. I implemented these checks in
this manner in order to provide error codes consistent with the
documentation. I am not incredibly familiar with the Wine internals,
but it looks to me like the functions called should handle a
simultaneous call from another thread (without unnecessary code
duplication). For example, if the "replaced" file goes missing between
the check and the CopyFileW call then CopyFileW will return an error.
If the "replaced" file appears between the DeleteFileW call and the
MoveFileExW call (since the MOVEFILE_REPLACE_EXISTING flag is not set),
then the MoveFileExW call will fail. The documentation's expected error
codes for ReplaceFile seem to be consistent with these potential
outcomes.
Reading MSDN that doesn't seem to be the case. The way I see it all
those errors documented could be caused by file permisions. MSDN doesn't
say anything about race cons.
Some notes about the patch:
+/* Maker sure the "replacement" file exists before continuing */
+if(!RtlDoesFileExists_U( lpReplacementFileName ))
+{
+SetLastError(ERROR_INVALID_PARAMETER);
+return FALSE;
+}
This would probably the right place to lock the file...
+/*
+ * If the user wants a backup then that needs to be performed first
+ * * Do not fail to backup if "replaced" does not exist
+ */
+if( lpBackupFileName && RtlDoesFileExists_U( lpReplacedFileName ) )
The call to RtlDoesFileExists_U() is redundant and introduces a race
con. CopyFileW() will check for existence, check GLE afterwards.
+{
+/* If an existing backup exists then copy over it */
+ */
+if(!CopyFileW( lpReplacedFileName, lpBackupFileName, FALSE ))
+{
+SetLastError(ERROR_INVALID_PARAMETER);
+return FALSE;
+}
+}
+/*
+ * Now that the backup has been performed (if requested), copy the
replacement
+ * into place (make sure we can delete the original file first)
+ * * Do not fail if "replaced" does not exist
+ */
+if(RtlDoesFileExists_U( lpReplacedFileName ) && !DeleteFileW(
lpReplacedFileName ))
Same here, DeleteFile() will check for existence.
+{
+SetLastError(ERROR_UNABLE_TO_REMOVE_REPLACED);
+return FALSE;
+}
+if(!MoveFileExW( lpReplacementFileName, lpReplacedFileName, flags ))
Couldn't you just get rid of the DeleteFile, check why MoveFileEx()
failed and get rid of the call to RtlDoesFileExists_U(lpReplacedFileName)?
+{
+SetLastError(ERROR_UNABLE_TO_MOVE_REPLACEMENT);
+return FALSE;
+}
+/* All done, file replacement successful */
+return TRUE;
Re: kernel32: Implement ReplaceFileA/ReplaceFileW
Erich Hoover wrote: Real Name: Erich Hoover Description: Implements the functions ReplaceFileA and ReplaceFileW in kernel32 (Bug #7544). Also provides conformance test code to ensure proper functionality. Changelog: kernel32: Implement ReplaceFileA/ReplaceFileW Your patch seems to introduce lots of race cons. It's possible these exist on Win32 too but can't we avoid them?
Re: Wine and XP Themes
Frank Richter wrote: WRT speed: themes usually use alpha-blending extensively; however, the speed of Wine's AlphaBlend() can almost be measured in geological terms; Why is that? Looking at it it seems to be using XRender - since theming doesn't require Aero and stuff what way would there be to speed it up? (I admit I didn't really read the code. :) Felix
Re: [PATCH] user32: do not call NULL message callback
Fabian Bieler wrote: The only way I know of to test this behavior would be to pass a NULL pointer as callback. Naturally this results in a segfault without this patch. Is this okay for a testcase? Use if(0) instead of todo_wine then (and let the second patch, which fixes the bug, remove the if(0)).
Re: [PATCH] user32: do not call NULL message callback
Fabian Bieler wrote: Windows silently ignores NULL pointers supplied to SendMessageCallback as callback function. A testcase would be nice. Felix
Re: New wintest executable is there (but there are some issues due to introduction of skip)
And while we're at it, there seems to be some misconfiguration of the httpd (for quite some time), as the following URL generates an 500 Internal Server Error: http://test.winehq.org/data/200702241000/2003_W2K3-SE-IE7/ntdll:error.txt
Re: test.winehq.com status
Hi. A new crossbuild appeared today but the site shows all tests failing. See http://test.winehq.org/data/200702241000/ Felix
Re: [1/2] comctl32: add WM_PAINT message sequence tests
James Hawkins wrote: If you're subclassing the header control and checking messages from the header control, then you need to add another message sequence, HEADER_SEQ_INDEX. Problem is I need to test whether the header msgs are sent between the WM_PAINT and WM_NOTIFY of the listview - right now the WM_PAINT/WM_NCPAINT/WM_ERASEBKGND _are_ sent - just not in between but after WM_PAINT processing (which causes the flickering). I agree that the "hdr" flag is kinda hackish, any idea how to properly abstract away what I'm trying to do? Felix
Message spy log viewer
Hi. I just figured I never told anyone (did I?) - a while ago I wrote a Ruby/GTK2 tool to parse +message logs (along with other channels mixed in) and display them in a treeview - makes debugging message related problems much easier. I suppose others wrote similar scripts for their own use - but maybe someone will find it useful nevertheless: http://users.holycrap.org/flexo/spy.rb Use like: $ WINEDEBUG="+message" wine foo.exe 2>log $ ruby spy.rb log Felix
Re: Rediffed patch from Rein Klazes:winex11.drv:Add pointer check in X11DRV_SetDIBits
Louis. Lenders wrote: + if( IsBadReadPtr( bits, nrsrcbytes)) return 0; AFAIK you should never use that function since it introduces a race con. Try using an exception handler around the memory access itself. Felix
Re: wineboot: Start items in StartUp folder on boot, includes security measures.
[EMAIL PROTECTED] wrote: And I have never advocated for putting this in the registry, my suggestion has always been to store these settings in a file outside the .wine/drive_c jail area that is accessbile via wine's Win32 API. Wine is *not* a sandbox. Any .exe run can make use of native (Linux) syscalls as it wants. Felix
Re: Add RegisterRawInputDevices stub implementation
Kovács András wrote:
+BOOL WINAPI RegisterRawInputDevices(PRAWINPUTDEVICE pRawInputDevices, UINT
uiNumDevices, UINT cbSize)
+{
+int i;
Please keep the indentation style of the original file.
Re: version: Constify a formal parameter of VerQueryValue{A|W}()
Andrew Talbot wrote: I've seen this function represented with the first two parameters constified. Is anyone aware of a more const-correct version in the field than the one Felix mentions? Sorry about that, seems they changed the declaration in the new headers - I misinterpreted your comment about the first parameter. What's the problem with it anyway? Just keep adding "const" to internal functions until the warnings disappear. :) Felix
Re: comctl32: move up-down msg seq tests into new 'msg' test
James Hawkins wrote: This is not what needs to happen. The generic message sequence testing code needs to be factored into msg.c, but the specific tests need to stay in each control's test file. Guessed so... How about putting it into include/wine/test.h to share the code with user32/msg? Wouldn't help object size but I don't think a shared lib would be worth it... Felix
Re: version: Constify a formal parameter of VerQueryValue{A|W}()
Andrew Talbot wrote: -BOOL WINAPI VerQueryValueA( LPVOID pBlock, LPSTR lpSubBlock, +BOOL WINAPI VerQueryValueA( LPVOID pBlock, LPCSTR lpSubBlock, This is wrong - in the PSDK headers it's LPSTR (the first parameter should be const though). Felix
Re: wineboot: Start items in StartUp folder on boot, includes security measures.
[EMAIL PROTECTED] wrote: This sounds almost perfect. I think the counterpoint raised by James Hawkins would be adequately addressed by adding a winecfg option as follows: Since we want winecfg to be gone in the long-term a registry key alone would probably do fine. Felix
Re: Comctl32 Status Bar Test
Alex Arazi wrote:
-ok(rc.top == 2, "Expected 2, got %d\n", rc.top);
-ok(rc.bottom == 21, "Expected 21, got %d\n", rc.bottom);
-ok(rc.left == 0, "Expected 0, got %d\n", rc.left);
-ok(rc.right == 50, "Expected 50, got %d\n", rc.right);
I didn't really look at your new tests but the second of those tests you
removed failed ("got 1" IIRC) on Windows (XP) because we don't create a
parent window (and because the 21 is wrong. Should be 22 IIRC) - native
comctl32 sometimes does some strange stuff when a control doesn't have
one - so the test should create it.
Felix
Re: Some thoughts about the ominous DIB Engine
Felix Nawothnig wrote: Pierre could speak to this better than I, but my understanding is that the Quartz driver does not need the DIB engine. Quartz (the Mac's imaging and windowing API) is capable of drawing to offscreen bitmaps directly, at color depths that need not match the display's. I suggested to do all the work server-side a while back: Uhm, dunno where I just got the idea that Quartz would feature network transparency. Just ignore me. I really need some coffee now.
Re: Some thoughts about the ominous DIB Engine
Ken Thomases wrote: Pierre could speak to this better than I, but my understanding is that the Quartz driver does not need the DIB engine. Quartz (the Mac's imaging and windowing API) is capable of drawing to offscreen bitmaps directly, at color depths that need not match the display's. This has implications for the design of the DIB engine, because it should be designed to allow the graphics driver to override/bypass it. I suggested to do all the work server-side a while back: http://www.winehq.org/pipermail/wine-devel/2005-July/038695.html Especially see: http://www.winehq.org/pipermail/wine-devel/2005-July/038703.html Now, in my opinion we shouldn't care about those apps but it's not up to me. :) Felix
Re: comctl32: tab: update the item mask after a TB_SETITEM (fixes bug #7345)
Mikołaj Zalewski wrote: Tab uses a mask to mark which fields are set but it new fields were set by SetItem the mask wasn't updated. Actually all members are always valid (for the internal structure at least). I already sent a patch which removes the mask. Felix
Undoc. comctl32 mem management functions
Hi. comctl32 exports (undocumented) Alloc() and friends which call LocalAlloc => GlobalAlloc => HeapAlloc since it's doesn't use any fancy LMEM / GMEM flags... so shouldn't Alloc() call HeapAlloc() directly? If Global/Local* behave different than Heap* on Windows - wouldn't it then be desirable to replace the Alloc()/etc. calls in our comctl32 (we use it all over the place) by Heap*? Reason I'm asking: Since it's not documented the Free(NULL) semantics are not clear (although it's safe in our implementation) - and I'm wondering if we really want to internally use undocumented functions with unclear semantics when there is an easy (documented) way. And besides - the comctl32 code is full of "if(p) Free(p);"... Felix
Re: msi: InstallPackage check for UI level must not disregard flags (updated)
Misha Koshelev wrote: So do you think I should add a define like INSTALLUILEVEL_NOFLAGS somewhere set to 0xf then? Shouldn't that be 0x7? I guess either one would work since the first flag is 0x20. Do I need to change it to 0x7 or leave as is? So would 0x1f or 0xfe1f. Doesn't make it correct and could fail if there are undocumented flags (that gap before 0x20 looks suspicious to me...) - and that's more likely than undocumented levels imho (and even with 0x7 we cover another 2). Felix
Re: msi: InstallPackage check for UI level must not disregard flags (updated)
Koshelev, Misha Vladislavo wrote: So do you think I should add a define like INSTALLUILEVEL_NOFLAGS somewhere set to 0xf then? Shouldn't that be 0x7?
Re: [PATCH] coverity: CID35: fixed wrong condition
Marcus Meissner wrote:
- if(nRelPos>=0) { /* if this or preceding row */
+ if(nRelPos<=0) { /* if this or preceding row */
while(nRelPos<=0) {
Shouldn't that become a "do { ... } while()" then?
No, since there is a return item; after the while () loop.
I meant replacing just the while(), not the if() while().
It's mostly a matter of style though as gcc most likely will compile it
to a do {} while() anyway.
Felix
Re: [PATCH] coverity: CID35: fixed wrong condition
Marcus Meissner wrote:
- if(nRelPos>=0) { /* if this or preceding row */
+ if(nRelPos<=0) { /* if this or preceding row */
while(nRelPos<=0) {
Shouldn't that become a "do { ... } while()" then?
Re: Getting Fallout 2 to work well
Joonas Koivunen wrote: The problem I have is that fallout2.exe takes all the available CPU power, optimizing that might make other old games work better too. I'm interested in the task, though I haven't got much of an idea how GDI or DirectDraw api's work and I'd need some pointers where to start solving this problem. See http://bugs.winehq.org/show_bug.cgi?id=5526 (at least that's what causing performance issues for Fallout 1 but IIRC they use the same engine) Felix
test.winehq.com status
Hi. Anyone knows what happened to test.winehq.com/data? There has been no new crossbuilds for quite some time now... Felix
Re: dsound/tests: The tests link with dsound.dll so use GetModuleHandle() instead of LoadLibrary().
Francois Gouget wrote:
This is a NOP, it just seems a bit cleaner to me.
-hDsound = LoadLibraryA("dsound.dll");
-if (!hDsound) {
-trace("dsound.dll not found\n");
-return;
-}
-
+hDsound = GetModuleHandleA("dsound.dll");
+ok(hDsound != NULL, "dsound.dll not loaded!\n");
No it's not, that causes test failure on native when DirectX is not
installed (NT4 out-of-the-box I believe?).
Re: Throwing in an idea (probably it was discussed before though)
John Smith wrote:
This will just allow developers to hide bugs in wine and slow
development even further, I thing it's a bad idea.
Ahem. To say it politely - I didn't get an impression that WINE
developers are desperately looking for unknown bugs to fix.
Because it's a tedious and boring task to narrow down those unknown bugs
in closed-source apps. And that's exactly why we ask you (since you got
access to the sources) to tell us what the application is trying to do
which doesn't work in Wine...
Actually fixing bugs is usually easy, tracking them down OTOH...
(we checked relevant portions of our code though for potential bugs and
found nothing)
Even if your application relies on undocumented behaviour ("potential
bugs" heh) it should work - if it doesn't that's a bug in Wine which
should be fixed.
And by the way:
Managed=N breaks focusing for everything on my box (bug #2149)... :-)
Felix
Re: excluding kernel32.9[78] from a relay log
Saulius Krasuckas wrote: I want to exclude this stuff. Adding "97;98" to "HKEY_CURRENT_USER/Software/Wine/debug/RelayExclude" string value doesn't help me. Adding "_EnterSysLevel;_LeaveSysLevel;97;98" does not help either. Suggestions? Try "kernel32.97;kernel32.98". Felix
Re: winspool.drv: Conformance-Tests
Detlef Riekenberg wrote:
I wrote a Commandline-Program for testing the Printing-API and found
many misterious behaviour.
While rewriting the conformance-tests for winspool.drv, how should we
handle this bugs / features ?
There are several types of undocumented behaviour:
a) The documentation is plain wrong and applications will most likely
depend on the documented behaviour (often hard to tell - an example
would be a function which is supposed to return the needed buffer size
but misbehaves under some conditions):
In this case test for the documented behaviour, comment it out using
#if 0 and add a note that foobar32.dll Version 1.2.3.4 does not work
as documented.
b) The documentation is plain wrong and it's likely that applications
using the API will rely on the undocumented behaviour (for example if
MSDN got the argument order wrong):
In this case test for the undocumented behaviour and add a comment
telling that this is undocumented.
c) The documentation is wrong, misleading or incomplete and although
applications should not rely on the undocumented behaviour you found one
which does (for example if a function reports failure instead of
segfaulting when it retrieves a bad pointer):
In this case test for the undocumented behaviour and add a comment
telling which application depends on this undocumented behaviour.
Any other undocumented behaviour should not be tested for at all.
Disclaimer: These are my personal guidelines, so treat them as such. :-)
Example for GetPrinterDriverDirectory (W/A):
Win98se and WinME do not check the requested level
Just check this on NT then, but avoid using GetVersion() - I'd suggest
to do something like:
ret = GetPrinterDriverDirectoryW(foo);
if(ret != ERROR_CALL_NOT_IMPLEMENTED)
{
/* Here you can test stuff which fails on 9x/ME */
}
else trace("GetPrinterDriverDirectoryW not implemented, skipping tests\n");
/* ... and here the stuff which successeds on both 9x/ME and NT */
I marked that as todo("windows").
Is this ok?
No. Until there are other third-party win32 implementations using the
WRT (Reactos?) we use just todo_wine, nothing else.
Another Example for GetPrinterDriverDirectory and Enum*:
On w2k, the the returned "number of bytes required" in the ANSI-Version
is the same as in the Unicode-Version.
I'd handle this like a).
We probably want our implementation to work as documented (since this is
most likely what apps will depend on) but we can't test it since it
would fail on Windows - so we comment out the test.
For GetPrinterDriverDirectory, the rest from the unicode-version of the
result is visible in the second part of the buffer.
(I Think, w2k fill the buffer with the unicode-version and convert
in-place to ANSI. Should we do the same ?)
I'm not sure what you mean here...
If the function works as documented but fills the rest of the passed
buffer with undefined stuff (it NUL terminates the first part, right?)
we shouldn't test for it unless we find an app which depends on it.
Felix
Re: Cursor Disappears and reappears
Kevin DeKorte wrote: Using Wine 20050725 and Lotus Notes 6.51 when I move my mouse to the database tabs or over the replication window the mouse disappears while I move over the tab or replicated databases and then reappears when I leave the list. The databases are highlighted, but having the mouse completely disappear is a little annoying. I don't remember it doing this with 20041202. As a follow up I checked this on my XP machine and on that machine the cursor changes from an arrow to a hand pointer. So maybe the cursor just did not load correctly. So can a patch be done that if the cursor being selected is bogus that it stays the arrow pointer. It's a regression which happenend between 20050628 and 20050725 affecting multiple applications - see http://bugs.winehq.org/show_bug.cgi?id=3165 Felix
Re: PATCH: GlobalAddAtomA check for invalid ptr
Marcus Meissner wrote: Yes, but please don't use the IsBad* functions, use a proper exception handler. Next try, is this what you imagined. The reason for using an exception handler is thread safety - to achieve that you have to put it around the ATOM_AddAtomA call. Felix
Re: Porting Device Drivers to Winehq
John Shillinglaw wrote: Thank you all for your help. I was vaguely aware that wine could not use the 95 drivers for my card, and developing pcmcia-cs / alsa drivers was my goal. Finding accurate, recent win 95 decodeing info/ tools on the net was difficult, and I knew that wine developers face the same problem in developing wine to support apps. FYI, the Win98 DDK is available online (for free) at MSDN somewhere. Felix
Re: Inconsistency/bug with GetProcAddress
Mike McCormack wrote: Yes it is. NTDLL and KERNEL32 are both mapped to always the same address on >=NT4 (in NT3 only NTDLL was at a fixed address however). KERNEL32 and NTDLL are special cases because they are loaded first when the address space is empty, and since the NT kernel has no address space randomization they will consistently end up at the same address. As far as I know, there's no *garantee* of that, it just happens to be that way. If you know otherwise, please point me to the relevant documentation. :) http://www.codeguru.com/Cpp/W-P/system/processesmodules/article.php/c5767/ says: > When creating a process, the loader on Win 2000, Win XP and Win 2003 > checks if kernel32.dll and user32.dll (their names are hardcoded into > the loader) are mapped at their preferred bases; if not, a hard error > is raised. In WinNT 4 ole32.dll was also checked. In WinNT 3.51 and > lower such checks were not present, so kernel32.dll and user32.dll > could be anywhere. Anyway, the only module that is always at its base > is ntdll.dll. The loader doesn't check it, but if ntdll.dll is not at > its base, the process just can't be created. It's a common technique to rely on the fact that exports from kernel32 reside at the same address to use CreateRemoteThread calling LoadLibrary for remote code injection (this is not what Vitaliy's App is doing here but the assertions are the same). Felix
Re: Inconsistency/bug with GetProcAddress
Mike McCormack wrote: Is there are any reason why this is happening? Any ways to fix this? Under Windows 98, you will always get the same address for the same export from a dll, however that's not garanteed under Windows NT (or Wine). Yes it is. NTDLL and KERNEL32 are both mapped to always the same address on >=NT4 (in NT3 only NTDLL was at a fixed address however). Felix
Re: test case demonstrating PeekMessage give up timeslices
Oliver Mössinger wrote: We changed our program to avoid the needless polling (only two PeekMessage are needet then). We add also the PM_NOYIELD. This gives back the reaction of the program if some other process consumes process time. But the speed is more reduced than windows. So we actually got a real world app which depends (depended, but we don't want apps getting fixed for Wine, right?) on PeekMessage not giving away timeslices? Felix
Re: Wine passes WGA test
James Courtier-Dutton wrote: IIRC it's checking the location of the old Wine config registry key. This has all moved as part of the config file removal (and as was planned long before WPA came around). No doubt they will update their WPA checks at some point... Would it be wiser to implement a wine feature to block a particular application from seeing a particular registry key. I disagree. If Windows has such a feature and/or it's somehow useful in other situations it could be implemented but doing this to get around their Wine detection is just silly - we couldn't win that race as there are zillions of methods to detect Wine and most of them can't be avoided (the first thing that comes to my mind which can't be worked around in userspace is doing a unix syscall for example). Felix
Re: WRT: Links and Timeout
Francois Gouget wrote:
On the WRT-Page "http://www.astro.gla.ac.uk/users/paulm/WRT/wrt.php";,
the link for "current status of Wine's conformance tests" is dead
("http://fgouget.free.fr/wine/tests-en.shtml";).
The list of tests was really a pain to manage by hand and as it has
grown a lot since then it would be much worse now. So I discontinued
that page a long time ago (more than a year).
However I believe that some people have put up automated replacements
based on the automated test framework. So pointing to this set of pages
would be nice (sorry I don't have the URL handy).
You mean http://test.winehq.com/data/ ?
Felix
Re: DSOUND: prevent assertion in mixer.c
Alex Villacís Lasso wrote: I guess this is preluded by some "length not a multiple of block size" errors? I've been experiencing those errors with the same failed assertion in another game and came up with a similar patch but didn't submit since I think this just hides another bug as it should *not* happen that buf_mixpos becomes greater than buflen (and this >= above should probably be ==) at any time. IMHO you should at least add an ERR to that branch. The "length not a multiple of block size" errors you mention is what happens *instead* of the failed assertion when this patch is applied. That's strange. I thought the failed assertion was caused by mixpos being increased by the fragment size rounded up to the next block size multiple... (the blocksize was 2 so mixpos was always buflen+1 when the assertion failed) Since the error of "length not a multiple of block size" is nonfatal, Yes it is fatal. Errors mean that something which should not have happened did happen - and we're not talking about something the API is supposed to handle... there is wrong code in Wine, internal states might have gone corrupted and you can no-longer be sure that Wine is working correctly. The difference to an assert() is that it's probably possible to repair the corrupted state but there is no guarantee for that since the error might be caused by completly unrelated code. And this is exactly what we you do in your code - you dunno the reason why mixpos became greater than buflen but you know how to handle it, that's why you should put an ERR there. unlike the assertion, I submitted the patch as it is. I will need to read more of both DirectSound and the source code in order to understand the implications of the non-multiple error, since there is another game from the same source (LittleWitch/FloatingFrameDirector) that displays the same non-multiple errors without triggering the assertion. What kind In the game I tested the assertion failure was not really reproducable, sometimes I played through the whole game (a demo) without problems - it also seems to be timing related... of setup or error needs to happen in an DirectSound application in order to trigger this non-multiple warning? Well, fragments of the wrong size are mixed in... but I dunno where they come from since I dunno anything about the DSound API. :-) And shouldn't it be "%= dsb->buflen;"? I'd think that this causes looping until new stuff is mixed in... I tested with the particular game, and it made no difference. The comment is there so it is evident that I tested both. Maybe new samples are immediatly mixed in? Felix
Re: Move Win16 relay messages to the relay16 channel
Uwe Bonnes wrote: Why? When you're debugging 16bit programs, sometimes you only want to see 16bit calls.:) Isn't your proposal similar to the recent "nrelay" proposal, where only calls from native code is logged, but not wine code calling other wine code? Why exactly do we need this (both nrelay and relay16) when there is RelayFromInclude? RelayFromInclude doesn't follow CreateProcess, if I understand things right... How about implementing magic values like %native% and %win16% for it then? "option-like" debug channels seem very unintuitive to me... Felix
Re: Move Win16 relay messages to the relay16 channel
Uwe Bonnes wrote: Why? When you're debugging 16bit programs, sometimes you only want to see 16bit calls.:) Isn't your proposal similar to the recent "nrelay" proposal, where only calls from native code is logged, but not wine code calling other wine code? Why exactly do we need this (both nrelay and relay16) when there is RelayFromInclude? Felix
Re: DSOUND: prevent assertion in mixer.c
Alex Villacís Lasso wrote: wine-pthread: mixer.c:386: DSOUND_MixInBuffer: Assertion `adjusted_remainder >= 0' failed. wine: Unhandled exception (thread 000a), starting debugger... WineDbg starting on pid 0x8 I guess this is preluded by some "length not a multiple of block size" errors? I've been experiencing those errors with the same failed assertion in another game and came up with a similar patch but didn't submit since I think this just hides another bug as it should *not* happen that buf_mixpos becomes greater than buflen (and this >= above should probably be ==) at any time. IMHO you should at least add an ERR to that branch. --- wine-20050725-cvs/dlls/dsound/mixer.c 2005-06-21 04:43:29.0 -0500 +++ wine-20050725-cvs-patch/dlls/dsound/mixer.c 2005-08-01 02:16:42.0 -0500 @@ -491,6 +491,7 @@ if (dsb->leadin && (dsb->startpos <= dsb->buf_mixpos)) dsb->leadin = FALSE; /* HACK: see above */ } + else dsb->buf_mixpos = 0; /* %= dsb->buflen; */ And shouldn't it be "%= dsb->buflen;"? I'd think that this causes looping until new stuff is mixed in... Felix
Re: test case demonstrating PeekMessage give up timeslices
I wrote: since a server call is much more expensive than a Windows system call. Would using shm fix that? No, you don't want to put the message queue in shared memory, that's not reliable. shm + kernel handles for synchronization then? Wait, putting the message queue into shm isn't what I wanted to suggest (although it would be possible with kernel handles, no?). :-) I meant that using shm generally would lower the cost of a server request and doing that extra yield would no longer be necessary (although we'd still have the other yield due to the request itself unless the queue is put into shm). Felix
Re: test case demonstrating PeekMessage give up timeslices
Alexandre Julliard wrote: since a server call is much more expensive than a Windows system call. Would using shm fix that? No, you don't want to put the message queue in shared memory, that's not reliable. shm + kernel handles for synchronization then? Felix
Re: test case demonstrating PeekMessage give up timeslices
Alexandre Julliard wrote: So, PeekMessage always yields execution (it shouldn't) with PM_NOYIELD specified it yields execution twice (although it shouldn't at all). PeekMessage is going to call the server and wait on the result, there's no way around it. The extra yield is to avoid hammering the server with requests in stupid apps that constantly poll for messages, But then that "extra" NtYieldExecution should not depend on !PM_NOYIELD since PM_NOYIELD doesn't have any effect on Windows, right? since a server call is much more expensive than a Windows system call. Would using shm fix that? Felix
Re: test case demonstrating PeekMessage give up timeslices
I wrote: So, PeekMessage always yields execution (it shouldn't) with PM_NOYIELD specified it yields execution twice (although it shouldn't at all). Err, that should read "and without PM_NOYIELD specified". Felix
Re: test case demonstrating PeekMessage give up timeslices
Alexandre Julliard wrote:
You can probably fix it by passing PM_NOYIELD in the PeekMessage
calls. But if your app needs a lot of CPU, restructuring the code to
avoid all the needless polling would give much better results, and
probably improve the behavior on Windows too.
That's just a workaround. Our PeekMessage is definitly misbehaving - I
ran the attached test-program in Wine and WinXP... here are the results:
Wine:
[EMAIL PROTECTED] c $ wine foo.exe
NtYieldExecution yielded 100 times
PeekMessage(...) yielded 200 times
PeekMessage(... PM_NOYIELD) yielded 100 times
WinXP:
C:\>foo.exe
NtYieldExecution yielded 100 times
PeekMessage(...) yielded 0 times
PeekMessage(... PM_NOYIELD) yielded 0 times
(The numbers slightly differ between runs for obvious reasons but they
are close enough (with an error margin of +/- 10 we could maybe make
this a real testcase))
So, PeekMessage always yields execution (it shouldn't) with PM_NOYIELD
specified it yields execution twice (although it shouldn't at all).
The (real) effect of PM_NOYIELD is described at
http://www.piclist.com/techref/os/win/api/win32/func/src/f67_6.htm
> You can optionally combine the value PM_NOYIELD with either
> PM_NOREMOVE or PM_REMOVE. However, PM_NOYIELD has no effect on 32-bit
> Windows applications. It is defined in Win32 solely to provide
> compatibility with applications written for previous versions of
> Windows, where it was used to prevent the current task from halting
> and yielding system resources to another task. 32-bit Windows
> applications always run simultaneously.
Felix
#include
DWORD (WINAPI *pNtYieldExecution)(void);
static INT nIdleSlices = 0;
static DWORD WINAPI ThreadProc(LPVOID lpParam)
{
for(;;)
{
nIdleSlices++;
pNtYieldExecution();
}
return 0;
}
int main()
{
HANDLE hThread;
DWORD dwThreadId;
HMODULE hNTDLL;
INT i;
MSG msg;
hNTDLL = LoadLibraryA("ntdll");
pNtYieldExecution = (PVOID)GetProcAddress(hNTDLL, "NtYieldExecution");
hThread = CreateThread(NULL, 0, ThreadProc, NULL, 0, &dwThreadId);
SetThreadPriority(hThread, THREAD_PRIORITY_IDLE);
for(i = 0; i < 100; i++)
pNtYieldExecution();
printf("NtYieldExecution yielded %d times\n", nIdleSlices);
pNtYieldExecution(); /* Reset scheduler queue */
nIdleSlices = 0;
for(i = 0; i < 100; i++)
PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE);
printf("PeekMessage(...) yielded %d times\n", nIdleSlices);
pNtYieldExecution(); /* Ditto */
nIdleSlices = 0;
for(i = 0; i < 100; i++)
PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE|PM_NOYIELD);
printf("PeekMessage(... PM_NOYIELD) yielded %d times\n", nIdleSlices);
TerminateThread(hThread, 0);
}
Re: LoadString problems
James Hawkins wrote:
Maybe that only returns the
hinstance of the process calling the module, and not the hinstance of
the module itself.
Ofcourse it doesn't do that, how should it know from what module the
call came from?
I didn't think so, so shouldn't GetModuleHandle(NULL) work? It leads
me to believe there's a bug in our GetModuleHandle.
Eh? Ofcourse GetModuleHandle(NULL) works - it retrieves a handle to the
calling process. This is what it does in Wine and what MSDN says about
it ("If this parameter is NULL, GetModuleHandle returns a handle to the
file used to create the calling process (.exe file).").
To make your code work it would have to retrieve a handle to the module
in which the used call to GetModuleHandle resides - but there is no way
it could know where that is since every call to the function resides at
a different address.
In theory you could roll back the stack to figure out EIP when the call
was made and then see what module is loaded at that address but that's
not what C APIs are supposed to do. And for that matter it's not what
Windows does either.
Felix
Re: [usp10] Add stubbed support for ScriptGetFontProperties
Oliver Stieber wrote:
+HRESULT WINAPI ScriptGetFontProperties(HDC hdc, SCRIPT_CACHE *psc,
SCRIPT_FONTPROPERTIES *sfp) {
+FIXME("%p,%p,%p\n", hdc, psc, sfp);
+/* return something sensible? */
+if (NULL != sfp) {
+sfp->cBytes= sizeof(SCRIPT_FONTPROPERTIES);
+sfp->wgBlank = 0;
+sfp->wgDefault = 0;
+sfp->wgInvalid = 0;
+sfp->wgKashida = 1;
+sfp->iKashidaWidth = 0;
+}
+
+}
You forgot the return value...
Felix
Re: LoadString problems
James Hawkins wrote: Maybe that only returns the hinstance of the process calling the module, and not the hinstance of the module itself. Ofcourse it doesn't do that, how should it know from what module the call came from? (Yes, there are ways. But they are hackish, wouldn't work always and this is not what MSDN says about this function anyway) The right way is to store the HINSTANCE passed to DllMain on DLL_PROCESS_ATTACH in a global variable and use that. Felix
Re: Benchmark's on the Wiki
Tom Wickline wrote: I'm not finished, but I thought I would ask if this going in the right direction? http://wiki.winehq.org/BenchMark Much better, although Windows should really be on the left side. Why? its Wine against XP ... not XP against Wine. Maybe it's just me (no it's not, someone else in this thread also suggested it ;) but when I looked at it I really expected XP being on the left side. This is probably caused by the third column - a green +42% looks like the expected increase when switching from Windows to Wine for me. Another explanation would be that we are reimplementing the Windows API - first there was Windows, then Wine was written. I'd also like to note that this (Windows, Wine) is the same order as used on test.winehq.com: 95,98,ME => NT3,NT4,2K,XP,2K3 => Wine (consider that this chronology is in terms of popularity with the families grouped afterwards). It also implies that Wine will take over the world one day. :-) Felix
Re: Benchmark's on the Wiki
Tom Wickline wrote: I'm not finished, but I thought I would ask if this going in the right direction? http://wiki.winehq.org/BenchMark Much better, although Windows should really be on the left side. Felix
Re: [wined3d] tidy up draw prim and add strided data new to d3d9
Oliver Stieber wrote: use ++x instead of x++ since x++ potentially requires a temporary variable. I seriously doubt this will actually have an effect on the resulting assembly code generated by any semi-serious compiler (regardless of the -O level) as for(i = 0; i < x; i++) is a quite common idiom... Felix
Re: KERNEL32: Re: Fix thread tests on WinME
Saulius Krasuckas wrote: Felix, I have reverted part of your patch and tested this. Reversion is doing fine on WinME -- zero failed tests. Didn't tested on XP or so. Is this acceptable? If so, feel free to submit it under own name. Or correct the code. :-) This is not really correct, SetThreadPriority, GetThreadPriority and GetThreadTimes all return ERROR_CALL_NOT_IMPLEMENTED on WinME. The order of the tests is wrong (first the restrictions are tested, then it's tested if they are actually implemented) - the tests for the former two functions need to be put into the scope below and the latter has to be put below the scope below. IIRC (I looked at it a few days ago). I'll do that tomorrow. Felix
Re: winecfg in 640x480
Joe Baker wrote: Well, in KDE use the Alt key and the left mouse to click and drag the window from anywhere in the window to move it higher. I think that blackbox supports this style of Window movement as well if you're on a lighter resource computer as your resolution suggests. This doesn't work in desktop mode (and probably non-managed mode) though. I'd also like winecfg to fit into 640x480. Felix
Re: Const Function Parameters?
Andreas Mohr wrote: While I'm not too convinced in this case (1.5% improvement sounds like within statistic noise), it should be a good idea to mark things in Wine const whenever possible (objdump -x helps here), since it improves reliability Huh? He did stuff like... -void foo(int i) +void foo(const int i) This will most likely not improve reliability. He also changed some pointers to const which is wanted and will ofcourse improve reliability. Assuming that gcc has a pretty good optimizer the only reason I can think of (besides a GCC bug :-) why performance increased is that gcc doesn't have to perform aliasing-analysis and can be sure that the dst pointer will never point into local parameters (constifying local variables could speed up things slightly more). I'm not sure about the "restrict" semantics but maybe it could be used instead of constifying "int i" to archieve the same effects? Felix
Re: dlls/winmm/winealsa/midi.c: Bug in MIDI_CMD_BENDER fixed
Johannes Koch wrote: No, you don't. The bitwise shifting operator has a higher precedence than the bitwise or does. In case you didn't know... $ man 7 operator is your friend. :-) Felix
Re: Const Function Parameters?
Michael Carlson wrote: I've also found that marking a couple of parameters of that function const (that I believe should be marked const anyways), CPU usage in that function drops measurably with oprofile. As far as I know, parameters that aren't modified in a function should be marked const anyways, to send the right hint to the compiler. Since it actually turns out to be faster too, it seems worth it to me. -static void convert_5x5_asis(int width, int height, - const void* srcbits, int srclinebytes, - void* dstbits, int dstlinebytes) +static void convert_5x5_asis(const int width, const int height, + const void* srcbits, const int srclinebytes, + void* dstbits, const int dstlinebytes) There is no need to make anything except the pointers const - I don't think I've ever seen that in real world code. In theory this would give the compiler slightly more information... but if the optimizer is unable to figure out that the parameter is never used as an lvalue by himself it sucks so badly that it probably won't do much better with that hints anyway. :-) Constifying the pointers is fine ofcourse (but rather because it helps finding bugs than for those 1.5% performance improvements. Felix
Re: commctrl: modularize progress drawing
Dimi Paun wrote: It has become the norm lately to not have the ALLCAPS_ prefix for internal static functions, but rather have names_with_underscores to easily tell them apart from Win32 APIs. So for the above, I guess get_led_size() would be preferable. I thought the only half-official rule would be to keep the style consistent with the rest of the file, which is just what Frank is doing here since progress.c uses ALLCAPS_CamelCase everywhere? And actually I'd prefer ALLCAPS_CamcelCase over kernel_style since the latter looks quite alien in Win32 code... and looking at wine-patches from the last weeks it seems I'm not alone. :) Felix
Re: Exception Handling with a "bad" ESP
Dimi Paun wrote: Direct memory access can be done using X11 SHM, no? But then you lose network transparency. I'd think that apps directly accessing the surface memory are either games or do some other kind of realtime graphics so network transparency isn't worth anything there (since we'd have to transfer the whole DIB all the time anyway). Felix
Re: Exception Handling with a "bad" ESP
Dimi Paun wrote: We need to get rid of that seperation - either the DIB should be stored fully on X side (which would require to add some features to X) or fully on Wine side (by implementing a GDI renderer in Wine). You can't have it fully on the X side, since you need direct memory access Direct memory access can be done using X11 SHM, no? (non-synchronized I might add) to the DIB data. MSDN says: Windows NT/2000/XP: You need to guarantee that the GDI subsystem has completed any drawing to a bitmap created by CreateDIBSection before you draw to the bitmap yourself. Access to the bitmap must be synchronized. Do this by calling the GdiFlush function. This applies to any use of the pointer to the bitmap bit values, including passing the pointer in calls to functions such as SetDIBits. (and I assume they are talking to application developers, not to Wine devs... :) Or is MSDN lying here? Felix
Re: Exception Handling with a "bad" ESP
Glenn Wurster wrote: Our current DIB implementation is an ugly hack. We need a real DIB implementation, either as a complete GDI engine in Wine or as an X11 extension. But is it the DIB implementation that is really at fault here or is it the fact that we are adding extra exceptions and can't be sure what the esp will be when they fire, especially since they don't occurr on Windows. Both are - the current DIB implementation wouldn't work without that extra exceptions. The "real" DIB, used for GDI calls, is stored on X side but the DIB surface, used for direct access, is stored on Wine side and we need to keep them in sync by catching reads / writes to the DIB surface. We need to get rid of that seperation - either the DIB should be stored fully on X side (which would require to add some features to X) or fully on Wine side (by implementing a GDI renderer in Wine). And since we already got a full-flegded rendering engine in X11 we already use (and will continue to use unless we wanna drop network transparency) I think the right way would be to add the missing features to X (which would just be XPixmaps of arbitrary depth as we should be able to use SHM for direct memory access). As far as I can tell, the problem is that we have extra exceptions firing when the application is not expecting them and they would not occurr on Windows. So, the two options are to enforce that Wine never throws exceptions other than those that occurr on windows (and I suspect people in general do not like this restriction for performance reasons) or to fix the exception handler to work in the face of a bad ESP. Perhaps exceptions fired due to wine can use a different stack so that they don't rely on an assumed state of the application? I dunno if that's possible but it would be yet another hack to workaround limitations of the current DIB hack (which also has other limitations that just *can't* be worked around). Some work has been done on a GDI engine in Wine by Transgaming a couple of years ago but it wasn't merged. Then, some weeks ago someone suggested to port the ReactOS GDI engine to Wine. Yes, I read that. The Transgaming engine that was discussed uses exceptions to detect when the application modifies a DIB, which is the problem we have right now. Does it? I see no reason for that... I don't know how the ReactOS GDI engine does it. I'm just talking about the render engine - a "real" GDI implementation doesn't need those exceptions (as you can observe on Windows). Felix
Re: Exception Handling with a "bad" ESP
Glenn Wurster wrote: Comments? Suggestions? Our current DIB implementation is an ugly hack. We need a real DIB implementation, either as a complete GDI engine in Wine or as an X11 extension. Some work has been done on a GDI engine in Wine by Transgaming a couple of years ago but it wasn't merged. Then, some weeks ago someone suggested to port the ReactOS GDI engine to Wine. However, in my opinion implementing a full-fledged GDI engine in Wine would be rather hackish and sub-optimal for several reasons: - It would need a lot of code and a lot of work. - The X11 engine is captable enough for most of our purposes (except this DIB thingie) so we shouldn't duplicate all that stuff which has been stable and working for decades unless it's absolutely necessary. - Sometimes using the X11 engine and sometimes our DIB engine for the same GDI functions could cause problems for some apps if the used algorithms differ slightly (probably more than a Wine/Windows discrepancy) - we could use our DIB engine for everything but this would eliminate network transparency. And it would be yet another hack. It would probably work but something like this should *really* be solved on X side using SHM and a yet-to-be-written extension IMHO. Comments? Suggestions? :) Felix
Re: [ntdll/tests/info.c] Skip test if needed
Paul Vriens wrote: every now and then one of the tests failed. Probably because a process is not longer there. You could spawn a child-process in the test which would be more reliable (or you could query the current process - that's what I do in the psapi tests)... Felix
