wxTreeCtrl->Delete deletes everything but the items 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.
FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

wxTreeCtrl->Delete deletes everything but the items

Post by FTC » Sun Jan 03, 2010 8:17 pm

Hi,

currently I am fighting with wxTreeCtrl. All I want to do is adding and removing items. Therefore I first stored the itemids in a list (from irrlicht), but when casting the iterator to an id and calling tree->Delete(id) nothing happend. So then I wrote a container class with only one wxTreeItemId attribute. Storing them into a list and using their ->getID() should work. But, well, it causes the strangest things to happen. Sometimes it does what it should and deletes the items from the tree, sometimes it just does nothing and sometimes it removes objects from the gui that do not even belong to the tree :shock:

To determine the Item to delete:

Code: Select all

        for(core::list<CTreeItemStorage*>::Iterator it_store = id_list.begin();it_store!=id_list.end();it_store++){
            CTreeItemStorage *tmpStore = (*it_store);
            wxTreeItemId tmpID = tmpStore->getID();
            scene::ISceneNode *tempnode = (scene::ISceneNode*)treeCtrl_ObjectManager->GetItemData(tmpID);
            if(tempnode == node){
                id_list.erase(it_store);
                treeCtrl_ObjectManager->Delete(tmpID);
                break;
            }
        }

Adding items is done like this:

Code: Select all

id_list.push_back(new CTreeItemStorage(treeCtrl_ObjectManager->AppendItem(treeItemTrack,wxString(name,wxConvUTF8),-1,-1,(wxTreeItemData*)tmpnode)));
I have no idea what could cause this?
I am using wxWidgets 2.8.10 on Win XP Sp3.

greetings
Attachments
weird_tree.jpg
This is what happens sometimes, when deleting items
weird_tree.jpg (23.36 KiB) Viewed 2432 times
normal_tree.jpg
this is how it should look like

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Sun Jan 03, 2010 11:02 pm

Uhm I have tried the wxTreeCtrl a little more and none of the Delete methods seem to work (DeleteChildren, DeleteAllItems)? Is there some method I have to call after deleting items?

greetings

Jorg
Moderator
Moderator
Posts: 3971
Joined: Fri Aug 27, 2004 9:38 pm
Location: Delft, Netherlands
Contact:

Post by Jorg » Mon Jan 04, 2010 7:47 am

Hi,

Are you absolutely positive that the ID you get back is a valid one? Try to check with wxTreeItemId::IsOk() and maybe also print them or debug. It might be that in the process of deep/shallow copying of your objects this property gets lost somehow.

What I usually do is not use the wxTreeItemId to be stored, but manually go over the tree and compare the pointer of the object that is going to be deleted compared to the pointer of the object in wxTreeItemData so that I can be absolutely sure it is the same object, and not pollute my object model with GUI specific classes like wxTreeItemId.

With regards,
- Jorgen
Forensic Software Engineer
Netherlands Forensic Insitute
http://english.forensischinstituut.nl/
-------------------------------------
Jorg's WasteBucket
http://www.xs4all.nl/~jorgb/wb

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Mon Jan 04, 2010 8:46 am

Ok I have checked in every possible way, even with invoking the first routine from here: http://wiki.wxwidgets.org/WxTreeCtrl

And the ID is ok, the Item is found, the data is valid until it comes down to:

Code: Select all

treeCtrl_ObjectManager->Delete(tmpID);
Sometimes it works alright, but sometimes it just stops there. It is like it just does not want to continue executing the code from there. The program does not crash or something, it just does not execute the rest of the function (and the rest of the function invoking this one).

I have never seen a behaviour like this? :?

Does this make any sense?

greetings

Jorg
Moderator
Moderator
Posts: 3971
Joined: Fri Aug 27, 2004 9:38 pm
Location: Delft, Netherlands
Contact:

Post by Jorg » Mon Jan 04, 2010 11:34 am

To get back at your code then...

Code: Select all

for(core::list<CTreeItemStorage*>::Iterator it_store = id_list.begin();it_store!=id_list.end();it_store++){
            CTreeItemStorage *tmpStore = (*it_store);
            wxTreeItemId tmpID = tmpStore->getID();
            scene::ISceneNode *tempnode = (scene::ISceneNode*)treeCtrl_ObjectManager->GetItemData(tmpID);
            if(tempnode == node){
                id_list.erase(it_store);
                treeCtrl_ObjectManager->Delete(tmpID);
                break;
            }
        }
Are you sure you can erase from the iterator while you are iterating it? I would be veruy careful with that.

Try to see if the objects get deleted when you do not erase them from the list right away but in a later stage.

With regards,
- Jorgen
Forensic Software Engineer
Netherlands Forensic Insitute
http://english.forensischinstituut.nl/
-------------------------------------
Jorg's WasteBucket
http://www.xs4all.nl/~jorgb/wb

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Mon Jan 04, 2010 3:28 pm

The iterator has nothing to do with it (it works on several other occasions), even with the recursive FindItem method I have the same problem.

I have now connected a method to the wxEVT_COMMAND_TREE_DELETE_ITEM event and the function is called everytime and the item is valid. But just refuses to get removed.

The code sometimes just stops executing there. That is another strange thing. Why does it simply stop inside the method? There is no return or something.

greetings

Auria
Site Admin
Site Admin
Posts: 6695
Joined: Thu Sep 28, 2006 12:23 am
Contact:

Post by Auria » Mon Jan 04, 2010 4:28 pm

Can you reproduce this in a minimal compilable sample? Maybe there is some memory corruption in your app or something hard to find
"Keyboard not detected. Press F1 to continue"
-- Windows

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Tue Jan 05, 2010 10:15 pm

Hm..
I have not been able to reproduce the exact same error, but the program crashes anyway (And I think I did everything right).

Maybe there is an obvious flaw in my programm which I am unable to identify.

http://rapidshare.com/files/330901066/t ... m.zip.html

greetings

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

Post by doublemax » Tue Jan 05, 2010 11:47 pm

I didn't check the whole code, but the following is very wrong:

Code: Select all

wxString nodestring = _("node")+wxString::Format(wxT("%i"),nodenum++);

// this code only happens to work because there are only ascii-characters in the string. But the length of the utf-8 encoded string can be longer than the number of characters in the string
char nodename[nodestring.length()];
strcpy(nodename, nodestring.mb_str(wxConvUTF8));

// main error here: cnode stores the pointer to a temporary char[]
cnode *node = new cnode(nodename,rand()%5);
Use the source, Luke!

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Wed Jan 06, 2010 6:35 am

So I can get the real size with sizeof(nodestring.mb_str(wxConvUTF8)) ?

On this wiki page it says:
mb_str() returns a temporary pointer. If you need to store it in a char* :

Code: Select all

 wxString mystring(wxT("HelloWorld"));
char cstring[1024];
strcpy(cstring, (const char*)mystring.mb_str(wxConvUTF8));
(I noticed I missed the (const char*) cast, but even with that, it does not work)
How do I get the char[] to be non-temporary?

van_user
Experienced Solver
Experienced Solver
Posts: 55
Joined: Wed Jun 11, 2008 9:28 pm
Location: UA

Post by van_user » Wed Jan 06, 2010 7:13 am

FTC wrote:How do I get the char[] to be non-temporary?
Use it like

Code: Select all

char* sm = new char( length)
. And free memory in destructor of cnode.

Jorg
Moderator
Moderator
Posts: 3971
Joined: Fri Aug 27, 2004 9:38 pm
Location: Delft, Netherlands
Contact:

Post by Jorg » Wed Jan 06, 2010 8:00 am

As something every C++ developer needs to know, when y you do this;

Code: Select all

void functionA() {
  
   SomeObject a;

   _pointerToObj = &a;
}
You will get in a 'heap' of trouble. Every object/string/char array/int etc you declare like this, will be on the STACK and the moment the function exits it will be gone. So if you store pointer references anywhere that can be accessed past the lifetime of this method, you are screwed.

If you want objects to continue living, either copy/clone the object with using operator= or a copy constructor.

In most cases I would avoid using pointers at all, try to use as little as "new" as possible becuase the memory management (cleaning up after yourself) is the one thing most developers are still getting bitten by.

Using operator=;
http://www.devarticles.com/c/a/Cplusplu ... -C-plus/2/

Using copy constructors;
http://www.fredosaurus.com/notes-cpp/oo ... ctors.html

So the above code can be either;

Code: Select all

void functionA() {
  
   _pointerToObj = new SomeObject();

}
Now the lifetime is guaranteed to surpass this method, BUT in the destructor you must delete this object yourself.

Also possible is;

Code: Select all

void functionA() {
  
   _instanceOfObj = SomeObject();

}
Which will do two things. First it will create a anonymous object of type "SomeObject", and invoke the operator= (default if not defined) and performs a shallow copy of all your class members. If you do not use deep references like pointers or references to other classes, it will be ok not to override operator= ..

I hope this helps a bit to clarify why it is a bad thing to refer to temp objects..

With regards,
- Jorgen
Forensic Software Engineer
Netherlands Forensic Insitute
http://english.forensischinstituut.nl/
-------------------------------------
Jorg's WasteBucket
http://www.xs4all.nl/~jorgb/wb

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

Post by doublemax » Wed Jan 06, 2010 9:17 am

Do you intend to have hundreds of thousands of cnode objects? If not you should just use wxString inside cnode and save you all the trouble just for saving a few bytes of memory.
Use the source, Luke!

FTC
Earned a small fee
Earned a small fee
Posts: 17
Joined: Tue Dec 15, 2009 10:00 am

Post by FTC » Wed Jan 06, 2010 12:15 pm

Alright, (I hope) I have corrected all syntax specific flaws:
http://rapidshare.com/files/331163899/t ... m.zip.html

Still it crashes :(
(Thanks for the pointerexplanations, I never really got the hang of that).

doublemax: I have changed that, but in the original program, I am working with Irrlicht which's nodes return an irrlicht specific string type that can be casted from or to a char pointer.

greetings

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

Post by doublemax » Wed Jan 06, 2010 2:34 pm

Code: Select all

m_treeCtrl2->AppendItem(parent,nodename,-1,-1,(wxTreeItemData*)pnode);
The last parameter is not a generic void* where you can store any arbitary pointer.
You must derive your class from wxTreeItemData if you want to store its pointer there.

After you changed that, you must also change:

Code: Select all

m_listBox3->Append(nodename, pnode);
to:

Code: Select all

m_listBox3->Append(nodename, (void *)pnode);
Otherwise the object will be deleted twice.
Use the source, Luke!

Post Reply