 |
P.S the latest master currently needs the usual round of trivial Linux fixes for a couple missing includes, but didn't want to toss it into the middle of what you were working on :-) |
 |
Hm, this stracktrace looks familiar, I think already fixed the same or very similar issue. It's caused by the optimistic approach of the wxGTK implementation posting idle events to expand ancestors, ignoring the possibility that the wxDataViewItem might actually be invalid by the time the idle event kicks in. There's no way to clear that event or clear the item unless somebody calls EnsureVisible(wxDataViewItem(nullptr)) after changing the model. |
|
|
I've done some more digging and I believe you are correct. I'm 100% certain I'm still experiencing 0005498, even after your fix was applied. I can prevent the crash by removing the "LastShaderClipboardMaterial" key/value from the map.darkradiant file. I see your `EnsureVisible(wxDataViewItem(nullptr))` that you added to mitigate this issue getting called early on, but if I put a breakpoint on that line and also on both `EnsureVisible(item)` lines in ResourceTreeView.cpp, I see the the latter getting called last, after the map is loaded and rendered but before the crash. I assume this would lead to that idle event getting reactivated and causing the crash. If you can't reproduce 0005498 in the latest master when opening a map that contains a "LastShaderClipboardMaterial", let me know and I can send you the stack trace showing where `EnsureVisible(item)` is getting called. |
|
|
There must be a code path I missed when placing that fix calling EnsureVisible(null). If ResourceTreeView.cpp:EnsureVisible is called last, there must be something happening in between that removes the model from the view (or clears the items). Can you maybe figure out if the AssociateModel() method is invoked after the item is scheduled by EnsureVisible(item)? Thing is, the MediaBrowser is reloaded once a map is finished loading. The ShaderClipboard notifier (LastShaderClipboardMaterial) schedules an item selection in the MediaBrowser, which is cleared almost immediately afterwards by the repopulator (at least I think that's the trigger) and the scheduled item is no longer valid. We need to figure out why the fix in AssociateModel is not enough in this case. |
|
|
If I load a map without the LastShaderClipboardMaterial, I do see AssociateModel() getting called, presumably as you described. However, it happens as a result of ResourceTreeView::_onTreeStorePopulationFinished and that is a callback for a WX event. I imagine the the expand ancestors event is somehow getting scheduled ahead of the population finished event. I'll keep playing with this and let you know what else I find. |
|
|
I found the reason - it's the Clear() call that is happening before the populator thread is starting. I'll have to think about how to rearrange that whole thing to make it safe. To be honest, I don't like the wxGTK implementation of scheduling an idle event with a dataviewitem that might be out of date by the time it's processed. There's no wxDataViewModelNotifier listening for item removed events to cancel the callback if the item is stale. The real solution would be to not embed the actual pointers to our Node structures in the wxDataViewItem, since there's no way to know whether the pointer is valid or stale. Some other ID that can be checked for validity would be nice, but this is of course some more work. The cheap solution of placing an EnsureVisible(null) in ResourceTreeView::Clear() would fix this particular crash, but it's not even close to be waterproof. |
- Anonymous
