>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.

Thread view

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.

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/

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.

>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';
}

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.

>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.

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.

>>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.

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.