kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 08/12/2013 01:29 PM, Tomasz Wlostowski wrote:
> On 08/12/2013 04:24 PM, Dick Hollenbeck wrote:
>>
>>
>> Unfortunately, I guess I am losing my hope that we have a unified vision.
>>
>> We apparently do not.
>>
>> The project is based on wx.  The portions of wx that are objectionable, to me are:
> 
> Dick, Wayne,
> 
> I said I'd prefer to not use wxEvtHandler for deriving the tools from, 
> but the tool framework code is at a proposal stage, so everything can be 
> changed.
> 
> I did my own event system, because I wanted to test some concepts (such 
> as FSM state switching and coroutines) which are orthogonal to any UI 
> library. I'm not a wxWidgets guru, and perhaps most of my requirements 
> could be met with some hacking of the wx's event system - I'm asking for 
> your help here. The reasons why I didn't use wxEvent/wxEvtHandler right 
> away are below:
> 
> - wxEvtHandler specifies a single method for handling events:
> 
> virtual bool ProcessEvent().
> 
> My idea was to have states in separate methods. One reason is avoid 
> enumerating states (and a big switch() statement in 
> MY_TOOL::ProcessEvent()), another is that with states in methods, FSMs 
> could be easily specialized by inheritance. I guess it should be 
> possible to do the same using a wxEventHandler. The base tool class 
> derived from wxEvtHandler could implement its own ProcessEvent() with 
> reworked dispatching code from TOOL_MANAGER, that calls the actual state 
> handlers and/or wakes them coroutines.

I cannot see how coroutines would work in a wx GUI program. Coroutines by definition are
the same OS thread under a second or more CPU stack region.  So we are talking about one
thread mimicking multiple.  Or are you using a contorted definition?  No way coroutines
will work in a wx program IMO.


The goal of the tool event handler should be to cooperatively return as fast as it can,
since it is being called on the wx event handler thread.  If you cannot process the event
and cooperatively return as fast as you can, you need a separate wxThread to do on going
work.  This, at least if you want the user to continue to see quick interactive behaviour.
 If you were doing a print run, where the user might have lower expectations on
responsiveness, then you don't need the wxThread helper thread.  If the wxThread also had
its own wxEvtHandler, you can queue wxEvents through there, so it becomes a way to
dispatch work tasks to a helper thread, say long compute runs, or whatever.  The two
threads could simply use wxEvents to talk to each other, each by poking something into the
others event queue.  Let's hope wxThread helpers are not often needed.  Most tools won't
need them.


If you were to duplicate the pcbnew behaviours we have now under the new tool
architecture, your tools would mostly all have one state in them.  The act of changing
tools would be the main state machine.  You only need more than one state, within the same
tool, when you plan on doing something different with the same kind of event.  If you
always intend to do the same thing with any given event type, you do not need more than
one state in a tool.  The tool can be thought of as a sub-state machine.  The tool change
is the topmost state machine.

I have used member function pointers and switch statements to put a state machine into a
class.  Here we are talking about 're-enterable' state machines, since we cooperatively
return as fast as we can after each event.  When we come back in to the tool via
ProcessEvent(), the tool can route execution to the state specific code.  When picking
between member function pointer or enums to identify states, enums are my preference,
because under a debugger if you have a variable that is an

enum STATE_T
{
}  m_state;


Then in the debugger m_state will always show with its *named* value.  No such joy with a
member function pointer.  If the enums are all sequential from zero, then the

 switch( m_state )

statement will be translated to an array like jump table by the compiler and this is only
basically two machine instructions.

The base tool class could be rigged to duplicate some of the event classification that
wxWindow::ProcessEvent() does.  This way you can at least get your mouse events and
keyboard events into to separate virtual functions, and derived tool classes would need to
override these if they want to do anything tool specific within these categories of
events.  It could also be a runtime decision as to whether the base tool class does the
classification and routes mouse events to OnMouseHandler(), or not.  A boolean to the
constructor could decide yeah or nay at the base tool class.

If you are going to use multiple states in a tool, and a OnMouseHandler(), and a
OnKeyHandler(), then you would need a switch( m_state ) in each overloaded virtual member
function.

There are certain aspects of wxEvtHandler that we do not need for a tool, such as the
message queue (except for the case of a helper wxThread).

I think if you hook any wxEvtHandler into the topmost frame, via the Push() call on the
wxFrame, then you are intercepting all wxEvents via your ProcessEvent() function.  It is
really quite easy.

A quick way to learn more about this is to outfit a wxEvtHandler derived class with event
printing functions, and simply hook it into a wxFrame in a test program.  If you do not
handle the event according to the wxEvent protocol (see wxEvent.Skip()) then wx passes
that event onto the next wxEvtHandler on the stack.
This may not be necessary

So much of the work is done.   I do think some duplication of wxWindow::ProcessEvent()
will need to happen in the base class tool.




> 
> - AFAIK wx uses integer IDs for defining UI commands. These IDs should 
> to be globally unique, which may be hard to meet, as the project grows 
> and many people contribute tool plugins.


Actually I cannot see where you even have UI ids any more.  Don't you just translate an
event into a an action call within the tool?



 I chose to call tools by name.
> I'm fine with wxEvents if we find a way to ensure that all IDs (also 
> across plugins) are easy to register and unique (hashes?).
> 
> - wx mouse events work in window space, tools work in board space:
> 
> I assumed that events passed to the tools will be adjusted to the 
> current state of the view. For example if the view is being 
> auto-scrolled (because the mouse is
> near the edge), the tools should receive a series of mouse motion events 
> because the cursor moves with respect to the world. Probably overloading 
> wxMouseEvent could fix it, but this brings extra complexity too. In wx, 
> mouse events are in screen space (GetLogicalPosition works for wxDC, but 
> not for the GAL which has a non-wx-related coordinate system) - so the 
> adjustment will have to be done in each state handler method or through 
> some global event filter. The former is not too elegant, and I don't 
> know if the latter is possible. You have much more experience with 
> wxWidgets than me - what do you think about this? Then, there is a 
> problem of distinguishing between events with board-relative coordinates 
> with plain wx mouse events that might be injected from other sources 
> (scripts). Also, tools should be able to request mouse snapping to 
> nearby objects/grid and. It could be done on event level, by adjusting 
> cursor coordinates accordingly.
> 
> - Drag vs click filtering:
> 
> Wx gives raw events. A click with a tiny mouse movement is considered a 
> drag, unless some filtering is applied, so an intermediate event filter 
> is required. I did it in TOOL_DISPATCHER class - maybe it could be 
> integrated into wx event stack and work on wxEvent derivatives?
> 
> - No idea how wx event stack will play with coroutines:


Again, getting me to believe in coroutines for a wxApp will require a brain transplant for me.



> One of the key points of my proposal was the ability to handle events in 
> a loop, 


Please elaborate, what that means.


a thing that we consider really useful and simplifying tool
> development (a small demo is in pcbnew/tools/selection_tool.cpp in 
> kicad-gal-orson branch). Is it possible to marry coroutines with the wx 
> event system? 

No, not even sensible in a wx program.


> It would be great if so, otherwise, me & Orson would miss 
> them very much (after all, P&S and interactive selection tools that we 
> are developing are the most complex ones, that really benefit from all 
> goodies of the tool framework...).

wxThread.


> 
> - Wanted to avoid translating between VECTOR2<>/BOX2<> and 
> wxRect/wxPoint in the tools.

Should be an operator overload.


> 
> - (a less technical one) I'm slightly biased against wx, so I didn't 
> want to depend too much on it:
>    * Different way of handling events on different plaforms: Let me give 
> an example - the selection tool opens a pop-up menu when multiple items 
> are clicked. We wanted to add some visual feedback that draws a bright, 
> contrast border of the item you're about to select from the 
> clarification menu. Then we discovered that it doesn't work on Windows: 
> under Linux, a menu choice event comes after a menu close event, on 
> Windows the order is opposite. I don't know if wx provides a mechanism 
> for filtering and reordering events and if it can be adapted to fix such 
> issues.

Again, you may have to duplicate some of wxWindow::ProcessEvent() or see if we can use a
hidden wxWindow as the base tool class.


>    * Mouse warping is forbidden on some OSes: emulate it (transparently 
> to the tools, so mouse events need to be adjusted somewhere) or drop it. 
> Existing Kicad tools use mouse warping quite frequently, so it's an 
> important issue.
>    * Linux issues: printing never, ever worked for me and the 
> never-ending story of incompatible versions, which gave me bad feelings 
> about general quality of the wxWidgets project and its packages in 
> different distros.


KiCad is based on wx, moving off of it would a multi man-year project.  I would have not
help.  Fighting wx makes the integration harder.  Learning to use it to your advantage is
the clever path forward.



Dick



> Regards,
> Tom