From: Jeremy W. <jez...@ho...> - 2005-10-17 20:05:24
|
While tracking various crashes down I found myself in the DoEvent function in GUI_Events.cpp, now I don't think there is a issue here - but I did notice something surprising. I would have through that if a control was using NEM events, the OEM logic wouldn't be called for that control. For example, a button has a NEM click handler, I wouldn't expect DoEvent to look for OEM events such as MouseMove for the same button - but it does. If this is correct, it would mean that to save doing a perl_get_cv call (which is really inefficient) you would have to define all possible events for all controls? Add this line: printf("EventName %s \n",EventName); After: // OEM name event char EventName[MAX_EVENT_NAME]; strcpy(EventName, "main::"); strcat(EventName, perlud->szWindowName); strcat(EventName, "_"); strcat(EventName, Name); To see the amount of needless calls made... Cheers, jez. |
From: Robert M. <rm...@po...> - 2005-10-19 18:57:58
|
Jeremy White wrote: > I would have through that if a control was using NEM events, the OEM > logic wouldn't be called for that control. That certainly should be the case. Unless you use the -eventmodel => 'both' option on the control, only one of PERLWIN32GUI_OEM and PERLWIN32GUI_NEN flags should be set for the control in perlud->dwPlstyle. > For example, a button has a NEM click handler, I wouldn't expect DoEvent > to look for OEM events such as MouseMove for the same button - but it > does. If this is correct, it would mean that to save doing a perl_get_cv > call (which is really inefficient) you would have to define all possible > events for all controls? > > Add this line: > > printf("EventName %s \n",EventName); > > After: > > // OEM name event > char EventName[MAX_EVENT_NAME]; > strcpy(EventName, "main::"); > strcat(EventName, perlud->szWindowName); > strcat(EventName, "_"); > strcat(EventName, Name); > > To see the amount of needless calls made... I haven't tried this yet, but it seems wrong. Can you raise a bug report and I'll dig further into this one. Thanks, Rob. -- Robert May Win32::GUI, a perl extension for native Win32 applications http://perl-win32-gui.sourceforge.net/ |
From: Jeremy W. <jez...@ho...> - 2005-10-20 10:29:20
|
>I haven't tried this yet, but it seems wrong. Can you raise a bug report >and I'll dig further into this one. http://sourceforge.net/tracker/?func=detail&atid=116572&aid=1333060&group_id=16572 Ok - it seems this is only an issue when SetEvent is used (see example below, also on tracker). What seems to happen is that the NEM event is called, and then it looks for an OEM event - if found that is ran too. Cheers, jez. --------------------------------------------- $|=1; use strict; use Win32::GUI; my $W = new Win32::GUI::Window( -name => "TestWindow", -pos => [ 0, 0], -size => [210, 200], -text => "TestWindow", ); $W->AddButton ( -name => "Start", -pos => [65,5], -text => "&Start", -tabstop => 1, #-onClick => sub {print 'clicked'}, ); #add the events to the button $W->Start->SetEvent('Click',sub {print 'clicked'}); $W->Show; Win32::GUI::Dialog(); |
From: Robert M. <rm...@po...> - 2005-10-20 17:07:49
|
Jeremy White wrote: >> I haven't tried this yet, but it seems wrong. Can you raise a bug >> report and I'll dig further into this one. > > http://sourceforge.net/tracker/?func=detail&atid=116572&aid=1333060&group_id=16572 > > Ok - it seems this is only an issue when SetEvent is used (see example > below, also on tracker). Ah. I'd expect this. When you create the control, as you don't have any NEM handlers set Win32::GUI chooses to use OEM. When you use $control->SetEvent() it sets the NEM flag, so you end up with both happening. You can work round this by: (1) using -eventmodel => 'byref' in the constructor. This explicitly forces NEM only. (2) by setting any -onEvent => $someref option in the constructor. Again this will force NEM only. (e.g. for your example -onClick => sub { return; } would be suitable) Question: is this a bug? The only alternate behaviour I can come up with is that SetEvent(), rather than setting the NEM flag, should check it and bail out if it is not set (with a warning?). [The current behaviour is useful if you are sub-classing a control, and do not know which method a user of the control will want to use.] My vote is to leave it as is. Regards, Rob. |
From: Jeremy W. <jez...@ho...> - 2005-10-20 19:16:49
|
>Question: is this a bug? The only alternate behaviour I can come up with >is that SetEvent(), rather than setting the NEM flag, should check it and >bail out if it is not set (with a warning?). [The current behaviour is >useful if you are sub-classing a control, and do not know which method a >user of the control will want to use.] Surely running both NEM and OEM code for the same event is a bug? In other words, a successful NEM call still causes the OEM logic to be run. If you happen to have an sub that matches, then you get two events being processed for a single event. See the code below for a better explanation:) I've just tried to follow the logic in DoEvent and the if statement that checks to see if we need to run OEM seems suspect (well, in the context of a NEM/SetEvent)? $|=1; use strict; use Win32::GUI; my $W = new Win32::GUI::Window( -name => "TestWindow", -pos => [ 0, 0], -size => [210, 200], -text => "TestWindow", ); $W->AddButton ( -name => "Start", -pos => [65,5], -text => "&Start", -tabstop => 1, #-onClick => sub {print 'clicked'}, ); #add the events to the button $W->Start->SetEvent('Click',sub {print 'clicked'}); $W->Show; Win32::GUI::Dialog(); sub Start_Click { print 'start click'; } |
From: Robert M. <rm...@po...> - 2005-10-21 00:07:09
|
Jeremy White wrote: >> Question: is this a bug? The only alternate behaviour I can come up >> with is that SetEvent(), rather than setting the NEM flag, should >> check it and bail out if it is not set (with a warning?). [The >> current behaviour is useful if you are sub-classing a control, and do >> not know which method a user of the control will want to use.] > > Surely running both NEM and OEM code for the same event is a bug? In > other words, a successful NEM call still causes the OEM logic to be run. > If you happen to have an sub that matches, then you get two events being > processed for a single event. See the code below for a better explanation:) Well, you could certainly argue that it is a bug, but I don't really know what the thinking was when this was originally written. There's certainly an option -eventmodel => 'both', which explicitly allows this (i.e. sets both the OEM and NEM flag). As I see the logic in DoEvents (psudeo-code): PerlResult = 1; // NEM event if((perlud->dwPlStyle & PERLWIN32GUI_NEM) && (perlud->dwEventMask & iEventId)) { PerlResult = 0; PerlResult = NEM_CALLBACK; ..... } // OEM Event if (PerlResult == 1 && (perlud->dwPlStyle & PERLWIN32GUI_OEM) && perlud->szWindowName != NULL) { PerlResult = 0; if there is a function called main::perlud->szWindowName_EventName { PerlResult = OEM_CALLBACK; ...... } } This says do the NEM event if the NEM flag is set and there's a coderef set for the event. Then do the OEM Event if: - Either there was no NEM event, or the NEM event returned 1 - And the OEM flag is set - And the window has a name - And the correctly name callback function exists So in my last analysis I missed the 3rd way not to get the OEM event fired: returning 1 from the NEM handler. I personally don't see anything wrong with this logic, so long as you believe that returning 1 from an event handler means 'pass the event to the next handler', regardless of whether this is another event handler, or defwindowproc. Although I note that DoHook doesn't implement such logic if you have multiple hooks for the same event. So, we come back to whether the SetEvent logic is right. Currently SetEvent replaces any NEM coderef for the event passed, or creates one if there wasn't one there before, and ensures that the NEM flag is set. As in my previous email, the only alternative I see is to get SetEvent to choke if the NEM flag is not set. I take back my position on not wanting to change this: having looked at my code I use hooks rather than SetEvent, in order not to interfer with what the user of my sub-classes intends. I am, thus, indifferent on making such a change, although I might argue that if you only want NEM, then you need to indicate that when you create the object. Do you have an alternative on what you think would be correct behaviour? Regards, Rob. |
From: Jeremy W. <jez...@ho...> - 2005-10-21 08:31:36
|
>So in my last analysis I missed the 3rd way not to get the OEM event fired: >returning 1 from the NEM handler. > >I personally don't see anything wrong with this logic, so long as you >believe that returning 1 from an event handler means 'pass the event to the >next handler', regardless of whether this is another event handler, or >defwindowproc. Although I note that DoHook doesn't implement such logic if >you have multiple hooks for the same event. I see your point. The problem is that most handlers don't explicitly return anything, which means we would always return 1. This in turn forces OEM logic to be run for any NEM event created by SetEvent. >So, we come back to whether the SetEvent logic is right. Currently >SetEvent replaces any NEM coderef for the event passed, or creates one if >there wasn't one there before, and ensures that the NEM flag is set. As in >my previous email, the only alternative I see is to get SetEvent to choke >if the NEM flag is not set. >I take back my position on not wanting to change this: having looked at my >code I use hooks rather than SetEvent, in order not to interfer with what >the user of my sub-classes intends. I am, thus, indifferent on making such >a change, although I might argue that if you only want NEM, then you need >to indicate that when you create the object. Do you have an alternative on >what you think would be correct behaviour? I would argue that the logic in SetEvent is wrong. My rational is as follows: There are two main reasons to use SetEvent: 1) To add NEM events to windows/controls after they have been created - perhaps by a 3rd party tool (such as Loft) which isn't NEM aware. 2) To change the event handlers during runtime. In both cases, the expectation should be that the control is now running under NEM only - just as if the control was created with NEM event handlers in the first place. If these assumptions are correct - and I could be wrong - shouldn't the approach be to simply turn off the OEM for a control that uses SetEvent? Instead of (in SetEvent): perlud->hvEvents = perlcs.hvEvents; perlud->dwEventMask = perlcs.dwEventMask; SwitchBit(perlud->dwPlStyle, PERLWIN32GUI_NEM, (perlud->dwEventMask != 0)); XSRETURN_YES; we have: perlud->hvEvents = perlcs.hvEvents; perlud->dwEventMask = perlcs.dwEventMask; SwitchBit(perlud->dwPlStyle, PERLWIN32GUI_NEM, (perlud->dwEventMask != 0)); SwitchBit(perlud->dwPlStyle, PERLWIN32GUI_OEM, 0); XSRETURN_YES; The control would now operate fully under NEM. Thoughts? Cheers, jez. |
From: Robert M. <rm...@po...> - 2005-10-30 17:50:37
|
Jeremy White wrote: >> So in my last analysis I missed the 3rd way not to get the OEM event >> fired: returning 1 from the NEM handler. >> >> I personally don't see anything wrong with this logic, so long as you >> believe that returning 1 from an event handler means 'pass the event >> to the next handler', regardless of whether this is another event >> handler, or defwindowproc. Although I note that DoHook doesn't >> implement such logic if you have multiple hooks for the same event. > > I see your point. The problem is that most handlers don't explicitly > return anything, which means we would always return 1. This in turn > forces OEM logic to be run for any NEM event created by SetEvent. But only if the object was created expecting to use OEM, so that the OEM flag is set, and as I have pointed out there are a number of way to ensure the OEM flag is not set at object creation time; if you do this, then OEM events are never looked for. >> So, we come back to whether the SetEvent logic is right. Currently >> SetEvent replaces any NEM coderef for the event passed, or creates one >> if there wasn't one there before, and ensures that the NEM flag is >> set. As in my previous email, the only alternative I see is to get >> SetEvent to choke if the NEM flag is not set. > > I would argue that the logic in SetEvent is wrong. My rational is as > follows: > > There are two main reasons to use SetEvent: > > 1) To add NEM events to windows/controls after they have been created - > perhaps by a 3rd party tool (such as Loft) which isn't NEM aware. > 2) To change the event handlers during runtime. > > In both cases, the expectation should be that the control is now running > under NEM only - just as if the control was created with NEM event > handlers in the first place. I don't follow this logical step. If a tool that is not NEM aware generates code, that presumably relies on OEM events being fired, and then I add so code that uses SetEvent(), your proposal would stop any existing OEM event handlers from being fired, even if for a different event. > If these assumptions are correct - and I could be wrong - shouldn't the > approach be to simply turn off the OEM for a control that uses SetEvent? I can't see that that is the right approach. If the object is created to use OEM, then SetEvent can have no idea whether there are actually OEM event handlers that need to be called. It does, however, know that after it is called there is at least one NEM handler to call. So all it can do is turn on the NEM flag. If you don't want OEM events fired, then ensure they are turned off when the object is created by setting at least one NEM handle, or by using the -eventmodel option. I guess I'm arguing that what's there is correct. Comments? Regards, Rob. |
From: Jeremy W. <jez...@ho...> - 2005-10-31 08:54:38
|
>>There are two main reasons to use SetEvent: >> >>1) To add NEM events to windows/controls after they have been created - >>perhaps by a 3rd party tool (such as Loft) which isn't NEM aware. >>2) To change the event handlers during runtime. >> >>In both cases, the expectation should be that the control is now running >>under NEM only - just as if the control was created with NEM event >>handlers in the first place. > >I don't follow this logical step. If a tool that is not NEM aware >generates code, that presumably relies on OEM events being fired, and then >I add so code that uses SetEvent(), your proposal would stop any existing >OEM event handlers from being fired, even if for a different event. My poor explanation. The tool wouldn't be responsible for generating the event handlers, just creating the window. For example, if you wanted to use Loft (or any GUI builder) and NEM, then you would use Loft to create the window and add the events later. Basically, I would have thought it would be valid to separate the step of creating a window, and associating events to it? >>If these assumptions are correct - and I could be wrong - shouldn't the >>approach be to simply turn off the OEM for a control that uses SetEvent? > >I can't see that that is the right approach. If the object is created to >use OEM, then SetEvent can have no idea whether there are actually OEM >event handlers that need to be called. It does, however, know that after >it is called there is at least one NEM handler to call. So all it can do >is turn on the NEM flag. If you don't want OEM events fired, then ensure >they are turned off when the object is created by setting at least one NEM >handle, or by using the -eventmodel option. Ok - if this is correct, then adding an event with SetEvent, shouldn't first call the NEM handler and still call the OEM handler for a single event. This is a bug. Returning 1 from the added NEM handler to stop this happening would be none standard. >I guess I'm arguing that what's there is correct. Comments? I can understand your point of view - and to some extent I agree with it. The issue is how to control the event model of a object post it's creation - there is no way to do this at the moment. I would have thought that using SetEvent would have explicitly changed event modes. As a compromise how about this: * SetEvent logic stays the same * The bug where 2 events are called for the same event when using SetEvent is fixed * A new method is added, which when called forces the object into NEM mode Cheers, jez. |
From: Robert M. <rm...@po...> - 2005-10-31 19:22:24
|
Jeremy White wrote: >>> There are two main reasons to use SetEvent: >>> >>> 1) To add NEM events to windows/controls after they have been created >>> - perhaps by a 3rd party tool (such as Loft) which isn't NEM aware. >>> 2) To change the event handlers during runtime. >>> >>> In both cases, the expectation should be that the control is now >>> running under NEM only - just as if the control was created with NEM >>> event handlers in the first place. >> >> I don't follow this logical step. If a tool that is not NEM aware >> generates code, that presumably relies on OEM events being fired, and >> then I add so code that uses SetEvent(), your proposal would stop any >> existing OEM event handlers from being fired, even if for a different >> event. > > My poor explanation. The tool wouldn't be responsible for generating the > event handlers, just creating the window. For example, if you wanted to > use Loft (or any GUI builder) and NEM, then you would use Loft to create > the window and add the events later. Basically, I would have thought it > would be valid to separate the step of creating a window, and > associating events to it? OK, I'm with you now. >>> If these assumptions are correct - and I could be wrong - shouldn't >>> the approach be to simply turn off the OEM for a control that uses >>> SetEvent? Even if the object was created with -eventmodel => 'both' ? >> I can't see that that is the right approach. If the object is created >> to use OEM, then SetEvent can have no idea whether there are actually >> OEM event handlers that need to be called. It does, however, know >> that after it is called there is at least one NEM handler to call. So >> all it can do is turn on the NEM flag. If you don't want OEM events >> fired, then ensure they are turned off when the object is created by >> setting at least one NEM handle, or by using the -eventmodel option. > > Ok - if this is correct, then adding an event with SetEvent, shouldn't > first call the NEM handler and still call the OEM handler for a single > event. This is a bug. Returning 1 from the added NEM handler to stop > this happening would be none standard. > >> I guess I'm arguing that what's there is correct. Comments? > > I can understand your point of view - and to some extent I agree with > it. The issue is how to control the event model of a object post it's > creation - there is no way to do this at the moment. I would have > thought that using SetEvent would have explicitly changed event modes. > As a compromise how about this: > > * SetEvent logic stays the same OK > * The bug where 2 events are called for the same event when using > SetEvent is fixed I don't know how to fix this. Within the event handler we don't know whether the OEM and NEM flags have been set explicitly or not. Are you suggesting that we shouldn't be testing the return value from the NEM handler before looking for the OEM handler? > * A new method is added, which when called forces the object into NEM mode (untried) Doesn't $window->Change( -eventmodel => 'byref' ); already do this? Regards, Rob. |