Before this patch, the timestamps recorded for DragDrop events in the win32 implementation were not comparable to the timestamps recorded for regular mouse or keyboard events. This was the case because it used In the DnD module (see Closes #509. How can I test this?For observing the consequences of this PR, please install this changeset into both an unpatched and patched VM and watch the outputs in the Transcript window while moving your mouse and dragging files from your Win32 host system over/into the image (works best when disabling the 'TranscriptStream forceUpdate' preference). Merger notesPlease review! You can view, comment on, or merge this pull request online at:https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/518 Commit Summary
File Changes
Patch Links:
— |
Ok, this is funny. On non-windows the — |
In reply to this post by David T Lewis
@krono It's just a theory, but an advantage of using — |
In reply to this post by David T Lewis
Makes sense; however, I think the Image expects io ticks… so I paged dave — |
In reply to this post by David T Lewis
The image simply expects an integer timeStamp relative to some (any) consistent start point. I expect no issues on the image side as long as events are milliseconds relative to the same basis. I am not a Windows expert, but the the Windows VM uses event timestamps from Windows events, and if those values are relative to system start time, then that is the way it should. Proposed patch as described above seems reasonable and good. As an aside, it might also be worth taking a look at ioMsecs() in the Windows VM to see if it could be changed to use the same time basis as Windows events (system startup time rather than VM startup time). If that could be done, then fixes like this would probably not be necessary. But don't let that suggestion stand in the way of integrating a good patch. — |
In reply to this post by David T Lewis
On a hunch I looked at ioMSecs() in the SVN repository, see line 1426 in the file at http://squeakvm.org/cgi-bin/viewvc.cgi/squeak/trunk/platforms/win32/vm/sqWin32Window.c?revision=2921&view=markup. This implements ioMSecs() as GetTickCount() &0x3FFFFFFF which is relative to system startup time. Thus the Windows VM originally had an ioMSecs() that was consistent with the time scale in the win32 MSG struct. Later changes to the platforms code may have changed this such that ioMSecs() no longer matches the time basis of the Windows events. — |
In reply to this post by David T Lewis
Sorry, I was citing ioMSec(), should have referenced ioMicroMSecs(). But in both cases, the time basis is system start time rather than VM start time, which is consistent with the time scale of a timestamps in a MSG struct. So in the earlier VMs we had: /* Note: ioMicroMSecs returns milliseconds / — |
In reply to this post by David T Lewis
Ok, I'll merge. @LinqLover, care to look at Daves proposal re — |
In reply to this post by David T Lewis
Merged #518 into Cog. — |
In reply to this post by David T Lewis
Thank you for the discussion and, first of all, for merging a working patch. :-) But it's true, — |
In reply to this post by David T Lewis
WinCE is dead, but yes, I'd say use that version but make sure the Mask is sensible. It is currently 32bit, isn't it? — |
In reply to this post by David T Lewis
To be clear - I do not propose reverting to the old version. But maybe there is a way to update the current version so that the time basis would match that of Windows events (time delta between system start time and VM start time is constant). It is just an idea, and I have not looked in detail. — |
In reply to this post by David T Lewis
David, this sounds interesting. Either we have to change the timestamps of all recorded events (possibly losing accuracy), or we change the reference value provided by So many questions! — |
In reply to this post by David T Lewis
@LinqLover on closer review I believe that the fix that you provided is the best solution. I see no other cases of event creation where this would be a problem. Good fix, no further action required. — |
In reply to this post by David T Lewis
So you mean there is no need to sync — |
In reply to this post by David T Lewis
I overlooked that. Let's take the question to squeak-dev and maybe loop tpr into the discussion. I do not know the answer. — |
In reply to this post by David T Lewis
A bit more history - For sqWin32Window.c, I compared the squeakvm.org versions versus initial checkins here, confirming that the timer changes were done by Andreas in work prior to first public release of Cog. Files attached for reference. I am completely confident that Andreas knew exactly what he was doing WRT event timestamps, so it seems to me the right thing to do is eliminate in the image any assumption that event timestamps match ioMSec(). I expect no that further changes should be necessary in the VM. — |
Free forum by Nabble | Edit this page |