Re: winex11.drv: Added missing mouse position clipping. (try 2)
Lauri Kenttä wrote: > My apologies for bringing up something that is actually none of my > business, but I think you should pay more attention to the way you write > your comments. First, even small positive comments are considered > psychologically very important, but you have given none. Second, most of > your negative feedback has only stated that the patches are bad and wrong, > often without giving much details or any better ideas. That is, the > comments haven't been very constructive. Currently your messages look a > bit like "f*** off, noob", which hopefully is not what you had in mind. > Anyway, this is certainly not a good way to encourage spending time to > Wine development. Some (luckily not me) would take it badly and just rm > -rf wine-git and never try again, even if they could be a great help with > some guidance. So let's all be nice to each other, and everyone will be > happier. > > Lauri: Programming can and is brutal. I deal with this daily and I've learned to keep a smile on my face, even when I'm being chastised. Drives folks 'nuts'. However, you have to learn the lesson, even if you win. One thing is that Windows does not trap the mouse and neither should Wine. If you are in a virtual desktop, the mouse should stop at the edge of the window and move to where you re-enter it. This is what you should strive for. If you cannot do this, then step back and take another try at it. I have been working on a fix for ONE function in Richedit for over one and one-half year now and the complaints still come in. However, I take them as constructive hints and work towards fixing and clearing them off. Soon, the patch will be submitted and I will move onto other problems that will rise because of the implementation of this code that are outside the scope of the code. That is the way it is in this business. James McKenzie
Re: winex11.drv: Added missing mouse position clipping. (try 2)
> The SetCursorPos() calls hooks on Wine because Wine has big issue - it
doesn't filter pointer move messages that were caused by Wine itself.
The first patch (http://source.winehq.org/patches/data/57569) tries to
address exactly this. I think XWarpCursor is not a problem, as the event
comes asynchronously with some delay. So my patch fixes the easier but
worse case, calling hooks directly from SetCursorPos.
What's wrong with this, why is it wrong, and do you have a better idea?
What else causes Wine to produce extra messages?
Also, is there a reason why I shouldn't write a test for this to make the
issue better known?
>> I'm aware that my version causes hooks to be called with clipped
coordinates on mouse move, when they should in that case be called with
the unclipped ones.
> This particular change will break all games using dinput when mouse
pointer goes outside of virtual desktop.
(This was the elaboration I was asking for.) I didn't know that, and I
don't have any such games. Feel free to reject the patch, I'll rewrite it
without this bug and in smaller chunks. (It would be helpful if the first
two patches were also either accepted or rejected.)
>> returns unclipped coordinates from GetCursorPos
> This is what native will do inside hook handler. Add some tests.
That's not true. (See below.)
>> and even sends incorrect WM_* messages.
> Tests please.
I've attached a program that outputs coordinates from hookproc and
wndproc, both the passed ones and the ones from GetCursorPos. If you try
it, you can see that Wine has the following bugs:
- incorrect coordinates in some WM:s
- incorrect coordinates in some hook calls
- incorrect coordinates and missing WM:s after clipping
- extra hook calls on SetCursorPos, even if the pos doesn't change
- extra WM_MOUSEMOVE after zero-length moves
- some other extra hook calls and WM:s due to XWarpPointer
The next snippets show the WM bug and how GetCursorPos works inside hook
handlers. This first one is from Win2k8 Server. First, the pointer is at
(34,34) and clipping is set to (30,30)-(40,40). Then the program calls
mouse_event(MOUSEEVENTF_LEFTUP | MOUSEEVENTF_MOVE, 0, 6, 0, 0).
11: hookproc: WM_MOUSEMOVE: (34,40), get = (34,34), injected = 1
11: hookproc: WM_LBUTTONUP: (34,39), get = (34,39), injected = 1
11: wndproc: WM_MOUSEMOVE: (34,39), get = (34,39)
11: wndproc: WM_LBUTTONUP: (34,39), get = (34,39)
As you can see, the correct order would be:
- call hooks for moving, return old pos from GetCursorPos
- clip and store the new position
- send the move message using the new position
- call hooks and send the button message, both using the clipped position
Compare to current Wine output:
11: hookproc: WM_MOUSEMOVE: (34,40), get = (34,34), injected = 1
11: hookproc: WM_LBUTTONUP: (34,40), get = (34,39), injected = 1
11: hookproc: WM_MOUSEMOVE: (34,39), get = (34,39), injected = 0
11: wndproc: WM_MOUSEMOVE: (34,40), get = (34,39)
11: wndproc: WM_LBUTTONUP: (34,40), get = (34,39)
11: wndproc: WM_MOUSEMOVE: (34,39), get = (34,39)
Aside of the extra move caused by XWarpPointer, the second hook and both
injected messages have incorrect (unclipped) coordinates.
> What exactly is wrong with [border scrolling]?
Many games (for example, Baldur's Gate) check for mouse coordinates. If
the pointer is outside the virtual desktop, Wine gives negative
coordinates and the game doesn't scroll left/up, because x != 0 and y !=
0. The same thing happens to right/bottom, of course. Correct clipping
fixes it.
> Who said that Wine should keep the pointer inside virtual desktop?
Ok, I'll not do it. But don't you think it's a bit confusing if the
pointer moves freely but the program thinks it's clipped to an area?
My apologies for bringing up something that is actually none of my
business, but I think you should pay more attention to the way you write
your comments. First, even small positive comments are considered
psychologically very important, but you have given none. Second, most of
your negative feedback has only stated that the patches are bad and wrong,
often without giving much details or any better ideas. That is, the
comments haven't been very constructive. Currently your messages look a
bit like "f*** off, noob", which hopefully is not what you had in mind.
Anyway, this is certainly not a good way to encourage spending time to
Wine development. Some (luckily not me) would take it badly and just rm
-rf wine-git and never try again, even if they could be a great help with
some guidance. So let's all be nice to each other, and everyone will be
happier.
--
Lauri Kenttä
#define _WIN32_WINNT 0x0500
#include
#include
int stage = 0;
RECT *tmp_rect(int x0, int y0, int x1, int y1) {
static RECT r;
SetRect(&r, x0, y0, x1, y1);
return &r;
}
INPUT *tmp_input(DWORD flags, int x, int y) {
static INPUT input;
input.type = INPUT_MOUSE;
input.mi.dwFlags = flags;
input.mi.dx = x;
input.mi.dy = y;
return &input;
}
const char *tmp_pos_s
cygwin, perl, wine, rebase, and ReBaseImage()
While building chromium with visual c++ 2005 on wine, step 'webcore_bindings_sources' tries to run WebCore/bindings/scripts/generate-bindings.pl, and sometimes it fails with 13 [main] perl 29 C:\chromium\src\third_party\cygwin\bin\perl.exe: *** fatal error - unable to remap C:\chromium\src\third_party\cygwin\lib\perl5\5.10\i686-cygwin\auto\List\Util\Util.dll to same address as parent(0x135) != 0x5276^M 14 [main] perl 74 fork: child 29 - died waiting for dll loading, errno 11^M http://www.tishler.net/jason/software/rebase/rebase-2.2.README indicates that cygwin's fork() relies on everything mapping to the same address in the child. (Gulp.) This is an occasional problem even on Windows; the usual advice is to run cygwin's rebaseall (carefully). Sure enough, running wine c:\\cygwin\\bin\ash c:\\cygwin\\bin\\rebaseall seems to have solved the problem. Or maybe I just got lucky. Cygwin's rebase command has its own implementation of ReBaseImage(), so it doesn't matter that Wine's is a stub. (Gee, maybe we should ask Ralf sometime if he's willing to relicense it from gpl. We'd have to translate it from C++ to C, too.) - Dan
Re: GSoC D3DX9 work
> Hello, > I might be interested as a side project. I've been poking around in the > wine code looking for something interesting for the last few weeks and > this might be it. However I know I wouldn't have much interest in it if > it doesn't actually help me, so I'd like to know a few additional > details about it. > > * Do you have list of bugs that your changes are known to fix? > * Same question for what bugs a completed work should fix. > * Could I see the TODO list to get a better idea of what needs to be done? > > Thank you. > (note that I replied to wine-devel since this might be useful for someone else as well) Okay, so here we go: Generally, I don't have visual (wine) tests for any of the stuff I wrote, not sure whether they're needed actually though. font code: - Probably the easiest to start with, although it might be a bit confusing if you're unfamiliar with the code. - Requires D3DXFilterTexture with D3DX_DEFAULT filter on A8R8G8B8 textures first though (that one's relatively easy to implement). - Requires this http://repo.or.cz/w/wine/d3dx9TW.git?a=commit;h=e4dfdb7c2cc4264bbe89ef938c939a3965f654ca patch to be applied first (fixes a bug in D3DXSprite implementation) - Not sure whether PreloadGlyphs was working correctly when I last worked on it. Glyphs are cached into textures and if an app caches to many glyphs new textures must be generated, which I didn't do first. I added this functionality but can't tell if it still works. Could be tested pretty easy with my testing demos. - Glyph positions in the texture cache (which are returned by GetGlyphData) differ by 1 or 2 from the native dll. At least they did, maybe I figured out why and fixed it meanwhile. It doesn't really matter anyways since it doesn't really affect applications, but just wanted to tell you ;) - two flags in DrawText aren't implemented, yet (DT_NOCLIP could be important to speed up things). Generally that one function (and its bunch of helper functions) really needs some cleanup before actually submitting it to wine-patches. It's forked from user32 code (or whereever DrawText is implemented in), so it's not /that/ clean. In conclusion, the only problematic bit here should be the last one. texture code, D3DXLoadSurfaceFromMemory: - implement other color conversion and filtering routines, you REALLY should talk to me in case you really want to do this, since I have thought much about how to organize code to be efficient and relatively clean as well, and I'm having a fairly good idea how it probably should be done. - What's missing for color conversion: about any format which is not unsigned 1,2,3 or 4 b(yte)pp ARGB, but my design allows fairly easy addition of support for large ARGB formats, signed formats, luminance channels and other non-compressed stuff. compressed formats might be hard, but not a necessarity to implement in the first place anyway. - other filters to be implemented: point filtering is the only one necessary here actually, since it can be used as a fallback for more advanced filters for now. texture code, other stuff: - not really much to fix here, it just all depends on a working D3DXLoadSurfaceFromMemory implementation. - only thing: my d3dx9 implementation implements W(indows)I(maging)C(omponent) codecs, and I'm not really sure how to integrate that one into Wine. right now they reside in the d3dx9 folder itself, but it could also be possible to create a new dll for it (since it might be usefull for d3dx10 as well)... you should talk to madewokherd (sorry if I'm misspelling that nick; he's the one who implemented most of the WIC stuff) and julliard (since he is the one to decide whether the integration method actually goes into mainline) on IRC about how to it properly I guess. mesh code: - right now dependent on d3dxof for .x file loading routines, but d3dx9 actually provides its own file loading routines, which should be reimplemented using d3dxof's methods. The current code must be ported over to using the d3dx9 methods. - Optimize() is not really implemented, I have a few ideas how to do it, but it's not of greates priority anyway - D3DXCreateMesh might work with some vertex declarations which do NOT map directy to a FVF, this might require architectural changes to the core code... - generally, the COM interfaces might not be implemented cleanly (maybe they are though, I wasn't sure how to do it properly since dsound and wined3d are doing this different, I went for the way wined3d does it). - .x file parsing doesn't read texture file names... this one might be trivial but I just couldn't get it working, for whatever reason - D3DXFVFFromDeclarator might not cover everything - I just forked it from some wined3d or d3d9 code (it does work fairly good at least) As for the affected bug reports, just search for the "d3dx" or "createtexture" terms on bugzilla ;) No non-DX-SDK apps are really dependant on the mesh stuff AFAIK, only few on the font stuff (e.g. the Dolphin emulator), so there aren't
comctl32 listview notification assert
Hello, I am seeing something in listview.c that really make me feel like there is something really wrong going on. So maybe it is something I am having a lack of understanding in. I have an application that is sending 'A' style notification codes to the listview header. Specifically I am seeing an HDN_ITEMCHANGEDA. In the listview code the first thing HDN_ITEMCHANGEDA and HDN_ITEMCHANGEDW does is notify_forward_header. notify_forward_header does not understand HDN_ITEMCHANGEDA and then asserts. It looks straight forward to add the A versions of the notifications to that case statement. But often when something looks that straight forward there is something more underlying it. Especailly since this looks like something we would be seeing a lot of if it was as much of an issue as it appears. does anyone have a deeper understanding of the listview code that would help me figure out what is going on. Or maybe just tell me that I am over thinking it and it really is as easy of a fix as it looks. thanks! -aric
Re: shell32: Dynamically load an ordinal only export.
On Fri, Jan 22, 2010 at 02:51:04PM +0100, Alexandre Julliard wrote: > Huw Davies writes: > > > On Fri, Jan 22, 2010 at 02:01:03PM +0100, Alexandre Julliard wrote: > >> Huw Davies writes: > >> > >> > --- > >> > dlls/shell32/tests/shlfileop.c | 18 +- > >> > 1 files changed, 9 insertions(+), 9 deletions(-) > >> > >> This shouldn't be needed, import by ordinal works just fine. > > > > I get a failure with make crosstest: > > You have to run make crosstest from the top level for the import > libraries to be built. Ah! Thanks! Huw.
Re: shell32: Dynamically load an ordinal only export.
Huw Davies writes: > On Fri, Jan 22, 2010 at 02:01:03PM +0100, Alexandre Julliard wrote: >> Huw Davies writes: >> >> > --- >> > dlls/shell32/tests/shlfileop.c | 18 +- >> > 1 files changed, 9 insertions(+), 9 deletions(-) >> >> This shouldn't be needed, import by ordinal works just fine. > > I get a failure with make crosstest: You have to run make crosstest from the top level for the import libraries to be built. -- Alexandre Julliard julli...@winehq.org
Re: shell32: Dynamically load an ordinal only export.
On Fri, Jan 22, 2010 at 02:01:03PM +0100, Alexandre Julliard wrote: > Huw Davies writes: > > > --- > > dlls/shell32/tests/shlfileop.c | 18 +- > > 1 files changed, 9 insertions(+), 9 deletions(-) > > This shouldn't be needed, import by ordinal works just fine. I get a failure with make crosstest: ../../../tools/winegcc/winegcc -b i586-mingw32msvc -B../../../tools/winebuild --sysroot=../../.. appbar.cross.o autocomplete.cross.o generated.cross.o progman_dde.cross.o shelllink.cross.o shellpath.cross.o shfldr_special.cross.o shlexec.cross.o shlfileop.cross.o shlfolder.cross.o string.cross.o systray.cross.o rsrc.res testlist.cross.o -o shell32_crosstest.exe -lshell32 -lole32 -loleaut32 -luser32 -ladvapi32 -lkernel32 shlfileop.cross.o: In function `test_shlmenu': /home/daviesh/wine/dlls/shell32/tests/shlfileop.c:2232: undefined reference to `_shell_mergeme...@24' /home/daviesh/wine/dlls/shell32/tests/shlfileop.c:2234: undefined reference to `_shell_mergeme...@24' Huw.
Re: shell32: Dynamically load an ordinal only export.
Huw Davies writes: > --- > dlls/shell32/tests/shlfileop.c | 18 +- > 1 files changed, 9 insertions(+), 9 deletions(-) This shouldn't be needed, import by ordinal works just fine. -- Alexandre Julliard julli...@winehq.org
Re: winmm: Add a bunch of new mmio tests which discover some bugs in mmio handling.
Dmitry Timoshkov writes:
> @@ -45,6 +45,20 @@ static DWORD RIFF_buf[] =
> 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
> };
>
> +#define expect_buf_offset(_hmmio, _off) \
> +do \
> +{ \
> +MMIOINFO _mmio; \
> +LONG _ret; \
> +memset(&_mmio, 0, sizeof(_mmio)); \
> +_ret = mmioGetInfo((_hmmio), &_mmio, 0); \
> +ok(_ret == MMSYSERR_NOERROR, "mmioGetInfo error %u\n", _ret); \
> +ok(_mmio.lBufOffset == 0, "expected 0, got %d\n", _mmio.lBufOffset); \
> +_ret = mmioSeek((_hmmio), 0, SEEK_CUR); \
> +ok(_ret == (_off), "expected %d, got %d\n", (_off), _ret); \
> +} \
> +while (0)
Please avoid such multi-line macros, define a function instead.
--
Alexandre Julliard
julli...@winehq.org
Re: [PATCH 1/2] ws2_32: use ntstatus in overlapped functions (try 2)
Mike Kaplinskiy writes: > Also some style fixes and move case 0 to the top - it's the most likely Nothing says that likely cases have to be at the top, and anyway 0 is probably the least likely of all (and should in fact never happen). -- Alexandre Julliard julli...@winehq.org
Re: "Mockba to Berlin" game - mouse not clicking
2010/1/22 Pavel Troller : > But the bug, together with other similar ones, has been closed as fixed in > 1.1.36, while this bug is still here. I cannot test the game with wine-1.1.34, It was closed as fixed because the specific game happened to start working, but the problem of ddraw switching to exclusive mode in the wrong place is still there.
Re: "Mockba to Berlin" game - mouse not clicking
> 2010/1/22 Vitaliy Margolen : > > Pavel Troller wrote: > >> BUT: The mouse buttons are DEAD. All I cannot left-click, right-click, > >> mid-click, simply nothing. > > Sounds like a new bug in wined3d. Try older Wine version(s) (wine-1.1.28 > > ish). > > > Yeah, if the game uses ddraw it's probably the same issue as > http://bugs.winehq.org/show_bug.cgi?id=21025. Hi! But the bug, together with other similar ones, has been closed as fixed in 1.1.36, while this bug is still here. I cannot test the game with wine-1.1.34, because it crashes and ends up in wine debugger. So it will be probably another issue. Are there any traces available to activate to further debug the problem? With regards, Pavel
Re: "Mockba to Berlin" game - mouse not clicking
2010/1/22 Vitaliy Margolen : > Pavel Troller wrote: >> BUT: The mouse buttons are DEAD. All I cannot left-click, right-click, >> mid-click, simply nothing. > Sounds like a new bug in wined3d. Try older Wine version(s) (wine-1.1.28 ish). > Yeah, if the game uses ddraw it's probably the same issue as http://bugs.winehq.org/show_bug.cgi?id=21025.
Re: [2/2] xmllite/reader: Basic input object creation on IXmlReader::SetInput()
On 1/22/2010 11:37, Paul Vriens wrote: On 01/21/2010 07:08 PM, Nikolay Sivov wrote: This patch depends on previously sent, forget it properly, sorry. Basic input object creation on IXmlReader::SetInput() Hi Nikolay, This one doesn't apply cleanly on top of: "xmllite/tests: Test query for supported interface sequence while creating IXmlReaderInput instance" Oh. Sorry, forgot another one. Will resend as series.
Re: [2/2] xmllite/reader: Basic input object creation on IXmlReader::SetInput()
On 01/21/2010 07:08 PM, Nikolay Sivov wrote: This patch depends on previously sent, forget it properly, sorry. Basic input object creation on IXmlReader::SetInput() Hi Nikolay, This one doesn't apply cleanly on top of: "xmllite/tests: Test query for supported interface sequence while creating IXmlReaderInput instance" -- Cheers, Paul.
Re: [PATCH 4/5] xmllite/tests: Add basic test structure for IXmlReader
On 1/22/2010 11:20, Paul Vriens wrote: On 01/21/2010 07:20 PM, Reece Dunn wrote: ?_? Hmmm. According to http://msdn.microsoft.com/en-us/magazine/cc163436.aspx, this should work (but then msdn isn't always right). Do you know what happens if the CoGetMalloc/IMalloc_DidAlloc call is not made? If I leave this out the test doesn't crash. Feel free to remove it. Shouldn't the IMalloc object be released? I thought it was just a pointer? There's nothing to release. It's a static global pointer. I'm not sure what the intention for this piece of code was btw. Was it to prove what allocated the memory (or rather what didn't)? I was to check that reader isn't allocated with default IMalloc - MSDN tells that passing NULL for imalloc parameter will use default IMalloc. For now we could remove this test out to stop crash cause xmllite is empty yet and more tests for this allocator are needed further.
Re: [PATCH 3/3] jscript: Add error handling to Array.reverse
Hi Piotr,
On 01/20/2010 05:23 PM, Piotr Caban wrote:
+if(FAILED(hres1))
+return hres1;
+
hres2 = jsdisp_propget_idx(jsthis, l,&v2, ei, sp);
+if(FAILED(hres2)) {
+VariantClear(&v1);
+return hres2;
+}
if(hres1 == DISP_E_UNKNOWNNAME)
-jsdisp_delete_idx(jsthis, l);
+hres1 = jsdisp_delete_idx(jsthis, l);
Coverity (CID 1022) correctly spotted that the last if() will never be
reached as we are now bailing out on a FAILED(hres1).
Could you have a look?
The same is true (me thinks) for hres2 but Coverity doesn't complain
about that one, strange?
--
Cheers,
Paul.
Re: [PATCH 4/5] xmllite/tests: Add basic test structure for IXmlReader
On 01/21/2010 07:20 PM, Reece Dunn wrote: ?_? Hmmm. According to http://msdn.microsoft.com/en-us/magazine/cc163436.aspx, this should work (but then msdn isn't always right). Do you know what happens if the CoGetMalloc/IMalloc_DidAlloc call is not made? If I leave this out the test doesn't crash. According to that MSDN article, the CoInitialize is not needed. Does removing it make it work on Vista SP0? Nope. But if that's the true the test should probably be changed to check for that? Shouldn't the IMalloc object be released? I thought it was just a pointer? IMalloc_DidAlloc returns an int instead of a HRESULT (not a big issue, just can be confusing if someone expects HRESULT codes for the value of hr -- doing hr != 1 looks odd). True, this could be written nicer. I leave that to Nikolay as he struggles through xmllite. Is the IXmlReader object valid (i.e. does a call to Read fail with an error that there is no input set, or does it crash)? XmlNodeType nodeType = XmlNodeType_None; hr = IXmlReader_Read(reader,&nodeType); ok(FAILED(hr), "IXmlReader_Read should fail!"); - Reece Haven't checked that last one as we found that CoGetMalloc/IMalloc_DidAlloc is the culprit. It still only happens on Vista SP0 and looks like a valid call sequence, not? I'm not sure what the intention for this piece of code was btw. Was it to prove what allocated the memory (or rather what didn't)? -- Cheers, Paul.
