Developers Notebook-Event Propagation

From WxWiki
Jump to: navigation, search

Event Propagation

Introduction

This proposal is divided into 2 parts, the part that is implemented and (almost) agreed on and the part that was discussed but no satisfying and workable implementation was reached. This page was created to record the ideas and remarks made on the second part.

  • First part (and original proposal) - add a function to wxEvent that would tell if the event should propagate or not and change wxEvtHandler::ProcessEvent() to check this function instead of wxEvent::IsCommandEvent(), in other words no longer state that only wxCommandEvent(s) can propagate. Due to this separation an important optimization can now be done when writing a custom ::ProcessEvent() function that forwards events to other windows, the optimization lie in the fact the new custom ::ProcessEvent() can now temporarily suspend propagation of the event.
  • Second part (a request from Vadim to extend the propagation system even further) - extend the propagation system so that a user can alter the default propagation state of event (for the complete app or only for all children of a certain window). This for example would mean that a user could request wxWidgets to propagate all mouse events propagate (the don't propagate by default). To achieve this we should have a way to alter the propagation stated somewhere between event construction and the actual propagation check in wxEvtHandler::ProcessEvent()...

See "<nowiki>[wx-dev</nowiki> Event propagation proposal"] for the complete event propagation proposal mail thread.

Relevant discussion snippets

  • First text snip that mentions the idea:
On Saturday, May 24, 2003 1:49 AM Vadim Zeitlin wrote:
 In fact, the idea is so simple that I can't believe we had never thought
 about this before (maybe we did and I forgot?): you surely know that people
 ask all the time about being able to forward mouse events or key presses or
 whatever else to the parent, right? Using ShouldPropagate() test instead of
 IsCommandEvent() one we could trivially make it possible!
 
  Just maintain somewhere a map<wxEvent kinds, bool> or even simply a list
 of event classes for which the propagation is on and allow people to turn
 it on and off during run-time as needed. Of course, the default behaviour
 would be the same as the current one.
 
 Am I missing some obvious reason this wouldn't work?


  • A follow-up explains the idea a bit better:
On Saturday, May 24, 2003 12:09 PM Vadim Zeitlin wrote:
<pre>

I'm afraid I didn't explain what I meant well as it seems Julian was confused by it as well. I'm trying to address a different problem: some people want to propagate the events other than the command ones _by_default_, i.e. they'd like wxMouseEvents to be propagated as well. I'd like them to give a possibility to change this if only to never have any discussions about why this should be possible on the lists any more :-) This is why I'd like to have a way to configure (from users code) the initial value of m_shouldPropagate (see point (1) above).

Julian has a good point however when he says that it is usually only

needed to change propagation for one window only. So maybe we should have a virtual wxWindow::GetDefaultEventPropagation(wxEvent&) which would be called on GetEventObject() in wxEvent ctor? By default GetDefaultEventPropagation() would return true if events type is a command one and false otherwise but it would be possible to override it in, for example, YourFrame class to return true for the mouse events as well. Argh, except that it assumes that parents method is called and not the one of the originating window... Hmm, maybe it's more subtle than I thought. I'm sorry, I don't have time for more now but I hope this gives you the idea of the problem I'm trying to solve. </pre>

    • Some valid remarks where given on the danger of allowing this
On Saturday, May 24, 2003 7:58 AM Stefan Csomor wrote:
<pre>

> > Am I missing some obvious reason this wouldn't work?

some thoughts

- we may depend internally in things like splitter windows etc on the fact that we do not get suddenly the mouse events from our children...

- I think we should reflect in which scope should a propagagtion change be valid, per event instance, per toplevel window class or instance, global ?

- could we think of a way to optimize the event handler speed by having cached maps for the first handler instance of a certain event kind ? </pre>

In a follow-up mail:
Saturday, May 24, 2003 8:02 AM Stefan Csomor wrote:
<pre>

> - we may depend internally in things like splitter windows > etc on the fact that we do not get suddenly the mouse events > from our children...

in order to solve this, it must be the parent window which can indicate that it is interested in its children mouse / key etc events. </pre>

      • Feedback on the remarks made by Stefan:
On Sunday, May 25, 2003 1:39 AM Vadim Zeitlin wrote:
<pre>

SC> in order to solve this, it must be the parent window which can indicate that SC> it is interested in its children mouse / key etc events.

I agree, now the question is how. Except for my (still not totally clear

even to me) idea of calling some virtual wxWindow::GetPropagation() I don't see any other way, do you? </pre>

        • A rough idea how to handle the blocking of events:
On Sunday, May 25, 2003 11:46 AM Hans Van Leemputten wrote:
<pre>

> see any other way, do you?

I have a idea, but it is of course *yet* another check to add in wxEvtHandler::ProcessEvent()...

The idea is to add a Hash table to wxEvtHandler (or beter only to wxWindow), this hash table would hold the event type ids it shouldn't propagate. So a user could register all event id's it shouldn't propagate... For a parant window to do that behind the sence it would need to override AddChild() and indecate there what event's shouldn't propagate... </pre>

    • Remark about the (global) map:
On Saturday, May 24, 2003 11:41 AM Julian Smart wrote:
<pre>

Quite a can of worms :-)

This map doesn't allow for changing one event at a time (for one single event object), as Hans requires to temporarily stop propagation while processing events for the focussed control.

Although, you could combine the map with accessors in the event object for allowing per-class and per-instance control. I'm still dubious about changing wxWidgetss event-handling semantics for entire classes... </pre>

      • Feedback on the map remarks.
On Sunday, May 25, 2003 1:42 AM Vadim Zeitlin wrote:
<pre>

JS> This map doesn't allow for changing one event at a time JS> (for one single event object), as Hans requires to JS> temporarily stop propagation while processing events JS> for the focussed control.

The map is just for changing the *default* propagation strategy. You could

still do whatever you want for any single event by directly calling StopPropagate().

JS> Although, you could combine the map with accessors JS> in the event object for allowing per-class and JS> per-instance control. I'm still dubious about JS> changing wxWidgets event-handling semantics JS> for entire classes...

People seem to want to do it oftne enough... 
But we definitely need a way to do it only for particular window or, to be

more precise, for all children of a particular window. </pre>

    • First attempt to produce some code that would solve both requests
On Sunday, May 25, 2003 12:06 PM Hans Van Leemputten wrote:
<pre>

I've uploaded a first attempt patch... See: http://sourceforge.net/tracker/index.php?func=detail&aid=743086&group_id=9863&atid=309863</tt>

Overview of the changes: - Added wxEvent::m_shouldPropagateLevel variable, this variable indicates how may levels it should max propagate. - Added a function to check if propagation is allowed "ShouldPropagate()" and also a function to stop propagation "StopPropagate()" - For initialing wxEvent::m_shouldPropagateLevel in a changeable way I added a static function pointer that is defiantly initialized with our "GetDefaultPropagationValue()" function.

Problems: - Because m_pGetPropagationValue gets called in the constructor of wxEvent it isn't yet possible to do "event.IsKindOf(CLASSINFO(wxCommandEvent))" or "event.IsCommandEvent()" in "GetDefaultPropagationValue()"... to solve that I need to call m_shouldPropagateLevel again in the wxCommandEvent constructor... </pre>

      • Relevant part of some feedback given by Julian:
On Sunday, May 25, 2003 12:33 PM Julian Smart wrote:
<pre>

Also this setting of callbacks seems to be getting really quite baroque and over the top. I'd prefer something simpler. We'd be giving too much rope for users to hang themselves by anyway.

So I'd agree with most of it, but maybe not the callback setting which seems to be adding too much complexity and inefficiency to the event loop. Maybe you could point to situations that we couldn't handle without these callbacks. </pre>

        • Agree on the fact function pointer shouldn't be used...
On Sunday, May 25, 2003 2:44 PM Hans Van Leemputten wrote:
<pre>

> So I'd agree with most of it, but maybe not the > callback setting which seems to be adding > too much complexity and inefficiency to the

Ok, so you find a bit to bulky... this was just the fastest way to do what Vadim wanted it to do that I could think of. Note: the callback is only used at event construction time to set the propagation value, the event loop doesn't call it at all.

> event loop. Maybe you could point to situations > that we couldn't handle without these callbacks.

Well as there wouldn't be a way to change the propagation level it wouldn't be possible for a user to say a wxMouseEvent should propagate by default...

I myself am not after this behavior, it just seems (some) other user do want this. Further more I don't understand why someone would want to do this for all events of a curtain type.

Hope I'm not boring you guy with yet a other idea; here is one to do it without the callback: - So no callback any more. Default propagation level is set to 0 and for command event it is set to MAX. - If a user now wants to alter the behavior (s)he should add a event handler to their wxApp class and do the following in there:

void MyApp::OnEventIWantToPropagate(wxEvent &event) {

   if (!event.ShouldPropagate() && event.GetEventObject() &&

event.GetEventObject()->IsKindOf(CLASSINFO(wxWindow)))

   {
       event.m_propagationLevel = wxMAX_EVENT_PROPAGATION;
       ((wxWindow *)event.GetEventObject())->ProcessEvent(event);
   }

}

Note: this is untested code! This is something a user could do now (but then change the m_isCommandEvent instead), also this doesn't give a user the impression that we support/encourage behavior like this. </pre>

          • Most relevant part of some feedback:
On Wednesday, June 04, 2003 12:59 AM Vadim Zeitlin wrote:
<pre>

HVL> > Also this setting of callbacks seems to be getting really HVL> > quite baroque and over the top. I'd prefer something HVL> > simpler. We'd be giving too much rope for users to hang HVL> > themselves by anyway.

I half agree but only because I don't like the idea of using callbacks. If

we had a simpler (but even more flexible and hence more dangerous) way to modify the propagation level of the events, it would be better. </pre>

Second snip
<pre>

HVL> ((wxWindow *)event.GetEventObject())->ProcessEvent(event); HVL> } HVL> } HVL> HVL> Note: this is untested code!

This is probably better than callbacks but it still doesn't allow to

achieve the initial goal of making it possible to propagate only the events of the given type and only for [the children of] the given window. Does anybody see a way to allow this? </pre>

            • More ideas/input:
On Wednesday, June 04, 2003 11:03 AM Roger Gammans wrote:
<pre>

> But using callbacks is dangerous, I still prefer my initial idea of having > a map event -> propagation levels.

I'm not sure why you don't like callbacks. Or is just you don't wnat to use function pointers which is understandable.

If this is the case havce you considered using a C++ closure implementation, eg, someting like:-

class wxClosure { virtual DoAction() = 0 };

At it simplest.

I did use a mini event framework based on this in one of my apps, and it worked very well. ( The events were not related to gui stuff they were at the data lvel not the display level )

Of corse we've onlu really moved the function pointer in to the virtual table, but we dispatch on virtual tables already.

> This is probably better than callbacks but it still doesn't allow to > achieve the initial goal of making it possible to propagate only the events > of the given type and only for [the children of] the given window. Does > anybody see a way to allow this?

What about addding a function PropagateEvent(wxEvent& ev,wxEvHandler* from) so when you propagete the event so you can make a desision here before passing in on, in most cases it will just call ProcessEvent(wxEvent& )

The only problem I see with the is that processEvent is virtual, so we can't accept the compiler to elide the function call, though it could use the tail-recursion optimisation.

So I suppose you could add the extra parameter (with NULL as default) to ProcessEvent() instead if we were worried about that overhead, which removes the overhead for those classes not using the feature.

The only thing I can see about this is it it require method override which is per-class thing not a per-instance thing. Although the from test can be per-instance.

I can then imagine a lot of code like this:- class MyWindow : public wxWindow { typedef wxWindow inherited; public: ... virtual bool ProcessEvent(wxEvent& event, wxEvHandler* from); ... private: wxWindow * m_child1; };

MyWindow::ProcessEvent(wxEvent& event, wxEvHandler* from) { if ( from != m_child1 ) { inherited::ProcessEvent(event,from); } else { ... } }

And apart from the subtle bug (;-) , puts the extra fucntion call back in for the cases whre normal propagation is wanted to to this window.

Dunno, otherwise. </pre>

              • Feedback on the idea/input:
On Thursday, June 05, 2003 1:29 AM Vadim Zeitlin wrote:
<pre>

RG> I'm not sure why you don't like callbacks.

They invariably lead to reentrancy problems in complex situations.

RG> Or is just you don't RG> wnat to use function pointers which is understandable.

I don't what to use function pointers neither but this is just part of the

reason.

RG> I can then imagine a lot of code like this:- RG> class MyWindow : public wxWindow { RG> typedef wxWindow inherited; RG> public: RG> ... RG> virtual bool ProcessEvent(wxEvent& event, wxEvHandler* from); RG> ... RG> private: RG> wxWindow * m_child1; RG> }; RG> RG> MyWindow::ProcessEvent(wxEvent& event, wxEvHandler* from) { RG> if ( from != m_child1 ) { RG> inherited::ProcessEvent(event,from); RG> } else { RG> ... RG> } RG> }

I don't understand how is this going to work. If the event is not

propgated upwards by default, it is never going to reach MyWindow::ProcessEvent() in the first place, no? </pre>

                • Clarify the idea a bit more:
On Thursday, June 05, 2003 10:39 AM Roger Gammans wrote:
<pre>

> We could look at it from scratch but I didn't understand what were ou > proposing to change in ProcessEvent() itself, if anything.

Remove the propagate testing (except the wxWS_EX_BLOCK_EVENTS )from ProcessEvent() and change the parent->GetEventHandler()->ProcessEvent() call to :-

parent->GetEventHandler()->PropagateEvent(wxEvent&, wxEvtHandler* from)

Put the Propagation testing then seperately in the new virtual wxEvtHandler::PropagateEvent(wxEvent&, wxEvtHandler*) Function.

I'm sure this isn't perfect, but it does give you the ability to distinguish directly injected event against propagated ones easily and see who has propagetd them to you.

It really depends does a object need to back decsision about which events it receives, or does it sender need to decide wether an event is propagated. The previous discussion didn't seem celar about this.

> Sorry, I may be just too sleepy right now but I really just fail to > see what you propose.

I know I was (too sleepy) when I made the last reply, anyway ot appears I made a thunko in the orginal example code which confused me as well.

The method shouwn should have been:-

       MyWindow::PropagateEvent(wxEvent& event, wxEvHandler* from) {
               if ( from != m_child1 ) {
                       return ProcessEvent(event);
               } else {
                       ...
               }
       }

</pre>

                • Confirmation that using callbacks in a event shouldn�t be done:
On Friday, June 06, 2003 8:59 AM Lindsay Mathieson wrote:
<pre>

> They invariably lead to reentrancy problems in complex situations.

Just to add my two bits, I agree with Vadim - apart from the rentrancy problems, call backs lead to problems with threading. Mapped Events can be used to transfer a event back to the main gui thread. </pre>

            • Some more code/mind dump to get the problem solved:
On Thursday, June 05, 2003 12:39 AM Hans Van Leemputten wrote:
<pre>

> achieve the initial goal of making it possible to propagate only the events > of the given type and only for [the children of] the given window. Does > anybody see a way to allow this?

Question: should this only count for the direct children of the given window or also for children's children (etc)? In other words would the following be any good?

/* virtual */ unsigned int wxWindow::GetEventPropagation(wxEvent& event) {

   if (eventtype in map)
       return map value;
   wxWindow *parent = GetParent();
   if (parent)
       return parent->GetEventPropagation(event);
   else if (wxTheApp)
       return wxTheApp->GetEventPropagation(event);
   else
       /* No parent so nothing to propagte to */
       return wxNO_EVENT_PROPAGATION;

}

/* virtual */ unsigned int wxApp::GetEventPropagation(wxEvent& event) {

   if (event.IsCommandEvent())
       return wxMAX_EVENT_PROPAGATION;
   else
       return wxNO_EVENT_PROPAGATION;

}

But who is going to call GetEventPropagation()... I don't think this is it, far from, it will be slow or won't work at all. </pre>

              • Feedback on the code/mind dump:
On Thursday, June 05, 2003 1:34 AM Vadim Zeitlin wrote:
<pre>

HVL> In other words would the following be any good? <snip code>

I hope so because it is very close to what I had in mind.

HVL> But who is going to call GetEventPropagation()...

wxEvtHandler::ProcessEvent(), of course.

</pre>

Summary

  • Goal: alter default propagation behavior of events, making it possible to propagate only the events of the given type and only for (the children of) a given window.
  • Don't use function callbacks, one of the reasons is that this can cause reentrance problems.
  • The system should have a low overhead, as most wxWidgets user will never use it.
  • Checks like IsCommandEvent() or event.IsKindOf(CLASSINFO(wxCommandEvent)) can't be done on a event in the ctor.
  • ...