wx 2.9.1 - I do not understand the event posting anymore Topic is solved

If you are using the main C++ distribution of wxWidgets, Feel free to ask any question related to wxWidgets development here. This means questions regarding to C++ and wxWidgets, not compile problems.
Post Reply
ddv
Knows some wx things
Knows some wx things
Posts: 25
Joined: Mon Dec 18, 2006 4:23 pm

wx 2.9.1 - I do not understand the event posting anymore

Post by ddv » Thu Sep 16, 2010 4:40 pm

Hi all!

I am currently testing a freshly built wxwidgets 2.9.1 and I do not understand the documentation concerning the events must be posted.
Until now, when I have a worker thread that post events to the main gui, for example a image loader thread that send the loaded image filename, I used:

Code: Select all

void ImageLoaderThread::OnImageLoaded(const wxString& imageFileName)
{
   ImageLoaderEvent evt(wxEVT_IMAGELOADER_IMAGEREADY,imageFileName);

   evt.SetId(_destWindow->GetId());
   _destWindow->AddPendingEvent(evt);
}
_destWindow is the wxWindow receicing messages.
ImageLoaderEvent is a wxNotifyEvent-inherited class.

From what I read in the doc, am I supposed to use this code ?

Code: Select all

   evt.SetId(_destWindow->GetId());
   _destWindow->ProcessWindowEvent(evt);
even if I don't understand why it is called ProcessWindowEvent and not PushWindowEvent or QueueWindowEvent, because I want the event to be process not immediately but when the window will have to process it.

Also I read in the doc stuff like wxPostEvent (which is not thread safe), wxQueueEvent (which is thread safe), wxThreadEvent (which is inter-thread safe ?), QueueEvent, and the fact that wxString::c_str() should be used to make a deep copy of a string to send in the event, but Clone() would be also important for this ?

Well, after some thoughts, I guess I can write my code like this:

Code: Select all

void ImageLoaderThread::OnImageLoaded(const wxString& imageFileName)
{
   ImageLoaderEvent evt(wxEVT_IMAGELOADER_IMAGEREADY,imageFileName);

   evt.SetId(_destWindow->GetId());
   wxQueueEvent(_destWindows, evt.Clone());
}
The Clone method of my custom event class ImageLoaderEvent does not make a deep copy of the string (the wxString = operator is used), but since I read that wxQueueEvent is safe to use with wxString members, this should be OK ?

Or do I need to pass imageFileName.c_str() instead of imageFileName in the ctor of ImageLoaderEvent ?

Any thoughts ?
Thanks.

EzPresso
In need of some credit
In need of some credit
Posts: 8
Joined: Tue Aug 26, 2008 7:45 am
Contact:

Post by EzPresso » Thu Sep 16, 2010 9:20 pm

You should always use QueueEvent methods within secondary threads IMHO. Why don't you just create your evt object on heap?
### EzPresso.ru - Daylighting Simulation Software Development ###
Ubuntu 9.04 / Windows XP Pro
GNUC / MinGW / wxWidgets 2.9.1

EzPresso
In need of some credit
In need of some credit
Posts: 8
Joined: Tue Aug 26, 2008 7:45 am
Contact:

Post by EzPresso » Thu Sep 16, 2010 9:38 pm

Doublechecked the wxWidgets docs:

Code: Select all

virtual void wxEvtHandler::QueueEvent(wxEvent* event) [virtual]
This method is similar to ProcessEvent() but while the latter is synchronous, i.e. the event is processed immediately, before the function returns, this one is asynchronous and returns immediately while the event will be processed at some later time (usually during the next event loop iteration).

Another important difference is that this method takes ownership of the event parameter, i.e. it will delete it itself. This implies that the event should be allocated on the heap and that the pointer can't be used any more after the function returns (as it can be deleted at any moment).

QueueEvent() can be used for inter-thread communication from the worker threads to the main thread, it is safe in the sense that it uses locking internally and avoids the problem mentioned in AddPendingEvent() documentation by ensuring that the event object is not used by the calling thread any more. Care should still be taken to avoid that some fields of this object are used by it, notably any wxString members of the event object must not be shallow copies of another wxString object as this would result in them still using the same string buffer behind the scenes. For example:

Code: Select all

            void FunctionInAWorkerThread(const wxString& str)
            {
                wxCommandEvent* evt = new wxCommandEvent;

                // NOT evt->SetString(str) as this would be a shallow copy
                evt->SetString(str.c_str()); // make a deep copy

                wxTheApp->QueueEvent( evt );
            }

### EzPresso.ru - Daylighting Simulation Software Development ###
Ubuntu 9.04 / Windows XP Pro
GNUC / MinGW / wxWidgets 2.9.1

ddv
Knows some wx things
Knows some wx things
Posts: 25
Joined: Mon Dec 18, 2006 4:23 pm

Post by ddv » Fri Sep 17, 2010 8:51 am

EzPresso wrote:You should always use QueueEvent methods within secondary threads IMHO. Why don't you just create your evt object on heap?
QueueEvent, when used in wxWindow, is a protected method.
From the doc, and from your response, I read that wxTheApp->QueueEvent is (must be ?) used. It seems strange to have only this solution of using a global variable to post event...
I will do this, but I don't understand why wxTheApp->QueueEvent has been chosed instead of using the myWindow->QueueEvent way.

By the way, I discovered that myWindow->GetEvtHandler()->QueueEvent() is compiling, would this working ? (I hate global variables :wink: )

Sorry for my insistence, but it seems to me that the documentation is not clear concerning the right way to send events, depending on the context...

Thanks.

User avatar
doublemax
Moderator
Moderator
Posts: 14319
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Post by doublemax » Fri Sep 17, 2010 10:02 am

I'm not using wx 2.9.x in production code yet, so i'm not 100% sure about this...

The new 2.9.x documentation about events is a little weird and regarding AddPendingEvent() and QueueEvent() i think it contradicts itself. It implies that using QueueEvent is threadsafe on its own, but you still have to ensure deep copies of all string instances yourself, so i don't really see a big advantage over AddPendingEvent. Additionally, the need to create the event object on the heap creates a worse runtime performance.

My personal opinion is: Using AddPendingEvent to send events from worker threads is still ok, if the Clone() method of your event class ensures deep copies of all wxStrings (and all other reference-counted objects).
Check the second part of this post for an example:
http://forums.wxwidgets.org/viewtopic.p ... 503#103503

If your code works fine on 2.8, it should work (and compile) on 2.9. The only change is:
Instead of using some_window_pointer->AddPendingEvent(), it's now recommended to use some_window_pointer->GetEventHandler()->AddPendingEvent().

If you want to be extra safe and to comply with the wx documentation, use some_window_pointer->GetEventHandler()->QueueEvent() (like you already found out yourself), but you still have to make sure that your Clone() methods creates deep copies of the strings.
Use the source, Luke!

EzPresso
In need of some credit
In need of some credit
Posts: 8
Joined: Tue Aug 26, 2008 7:45 am
Contact:

Post by EzPresso » Fri Sep 17, 2010 10:11 am

QueueEvent, when used in wxWindow, is a protected method.
Yes, but it is public in wxEvtHandler. And you are right. You may use myWindow->GetEvtHandler()->QueueEvent()

The documentation for wxWindow class states:
... ProcessEvent() itself can't be called for wxWindow objects as it ignores the event handlers associated with the window...
### EzPresso.ru - Daylighting Simulation Software Development ###
Ubuntu 9.04 / Windows XP Pro
GNUC / MinGW / wxWidgets 2.9.1

ddv
Knows some wx things
Knows some wx things
Posts: 25
Joined: Mon Dec 18, 2006 4:23 pm

Post by ddv » Mon Sep 20, 2010 11:57 am

OK. Thanks to both of you.

I will use:

some_window_pointer->GetEventHandler()->QueueEvent()

and will use a secured Clone function (I have my own custom event class).

User avatar
evstevemd
Part Of The Furniture
Part Of The Furniture
Posts: 2282
Joined: Wed Jan 28, 2009 11:57 am
Location: United Republic of Tanzania
Contact:

Post by evstevemd » Mon Sep 20, 2010 2:20 pm

Excuse me guys,

but your talk on deep copy and shallow copy have left me down town, in amazing flashlight :) I mean I'm confused with it. Could somebody explain to me what it means? Thanks
Chief Justice: We have trouble dear citizens!
Citizens: What it is his honor?
Chief Justice:Our president is an atheist, who will he swear to?
[Ubuntu 19.04/Windows 10 Pro/MacOS 10.13 - GCC/MinGW/Clang, CodeLite IDE]

User avatar
doublemax
Moderator
Moderator
Posts: 14319
Joined: Fri Apr 21, 2006 8:03 pm
Location: $FCE2

Post by doublemax » Mon Sep 20, 2010 2:40 pm

Assume this code:

Code: Select all

wxString s1 = wxT("some text");
wxString s2 = s1;
In this case you will have two wxString objects (s1 and s2), but internally they will point to the same memory block containing the actual characters ("some text"). This is called "shallow copy". This saves memory and - as string assignments occur pretty often in normal code - performance. Only if one of the string is changed, the actual string content will be copied.

But wxString is not thread-safe, so in multithreaded applications this can cause problems.

Therefore, when creating a string that you know will be accessed from different threads (e.g. if you set the string member of an event that will be sent from a secondary thread to the main thread), you must make sure that the string data between two strings is not shared. This can be done by the following code:

Code: Select all

wxString s1 = wxT("some text");
wxString s2 = s1.c_str();
This creates a "deep copy".
Use the source, Luke!

User avatar
evstevemd
Part Of The Furniture
Part Of The Furniture
Posts: 2282
Joined: Wed Jan 28, 2009 11:57 am
Location: United Republic of Tanzania
Contact:

Post by evstevemd » Mon Sep 20, 2010 8:33 pm

doublemax wrote:Assume this code:

Code: Select all

wxString s1 = wxT("some text");
wxString s2 = s1;
In this case you will have two wxString objects (s1 and s2), but internally they will point to the same memory block containing the actual characters ("some text"). This is called "shallow copy". This saves memory and - as string assignments occur pretty often in normal code - performance. Only if one of the string is changed, the actual string content will be copied.

But wxString is not thread-safe, so in multithreaded applications this can cause problems.

Therefore, when creating a string that you know will be accessed from different threads (e.g. if you set the string member of an event that will be sent from a secondary thread to the main thread), you must make sure that the string data between two strings is not shared. This can be done by the following code:

Code: Select all

wxString s1 = wxT("some text");
wxString s2 = s1.c_str();
This creates a "deep copy".
Simple, clear!
Big thanks!
Chief Justice: We have trouble dear citizens!
Citizens: What it is his honor?
Chief Justice:Our president is an atheist, who will he swear to?
[Ubuntu 19.04/Windows 10 Pro/MacOS 10.13 - GCC/MinGW/Clang, CodeLite IDE]

Post Reply