Re: [lmi] wx-2.9.5-rc1 regression: proposed patch(es)

[Vadim Zeitlin]

 Hello again,

 Here are some patches fixing the problem: first, the simplest and smallest
but not the most efficient patch implementing the recursive traversal of
all children:

----------------------------------------------------------------------------
commit c91b4372bedf4d340ef44ce4ede82613944cbda5
Author: Vadim Zeitlin <address@hidden>
Date:   Tue Jul 16 16:50:06 2013 +0200

    Conditionally enable all children of the current page recursively.

    Update all children of the current page, including grand-children, in
    MvcController::ConditionallyEnable(), instead of doing it only for the top
    level children as this doesn't include most controls any more with wxWidgets
    2.9.5 which creates these controls as children of the static boxes and not 
the
    page itself.

diff --git a/mvc_controller.cpp b/mvc_controller.cpp
index 6ba0efc..ff48815 100644
--- a/mvc_controller.cpp
+++ b/mvc_controller.cpp
@@ -215,9 +215,9 @@ void MvcController::Bind(std::string const& name, 
std::string& data) const
     return WindowFromXrcName<wxBookCtrlBase>(view_.BookControlName());
 }

-void MvcController::ConditionallyEnable()
+void MvcController::ConditionallyEnableChildren(wxWindow& win)
 {
-    wxWindowList const& wl = CurrentPage().GetChildren();
+    wxWindowList const& wl = win.GetChildren();
     for(wxWindowList::const_iterator i = wl.begin(); i != wl.end(); ++i)
         {
         wxWindow* pw = *i;
@@ -240,7 +240,14 @@ void MvcController::ConditionallyEnable()
             // Do nothing. Some windows don't have validators--for
             // example, most static controls.
             }
+
+        ConditionallyEnableChildren(*pw);
         }
+}
+
+void MvcController::ConditionallyEnable()
+{
+    ConditionallyEnableChildren(CurrentPage());

     // TODO ?? This is ugly. The real problem, as was speculated
     // above, is that ConditionallyEnableItems() is too slow. It is
diff --git a/mvc_controller.hpp b/mvc_controller.hpp
index 4dac68a..3ff96cb 100644
--- a/mvc_controller.hpp
+++ b/mvc_controller.hpp
@@ -428,6 +428,7 @@ class MvcController
     wxBookCtrlBase      & BookControl()      ;
     wxBookCtrlBase const& BookControl() const;
     void ConditionallyEnable();
+    void ConditionallyEnableChildren(wxWindow&);
     void ConditionallyEnableControl(std::string const&, wxWindow&);
     void ConditionallyEnableItems  (std::string const&, wxWindow&);
     wxWindow& CurrentPage() const;
----------------------------------------------------------------------------

 The slightly more efficient version which recurses only inside the static
boxes can be obtained by applying an additional patch on top of this one:

----------------------------------------------------------------------------
diff --git a/mvc_controller.cpp b/mvc_controller.cpp
index e06bb55..742fb89 100644
--- a/mvc_controller.cpp
+++ b/mvc_controller.cpp
@@ -241,7 +241,13 @@ void MvcController::ConditionallyEnableChildren(wxWindow& 
win)
             // example, most static controls.
             }

-        ConditionallyEnableChildren(*pw);
+        // We need to recurse inside the static boxes as most of our children
+        // are actually inside them and not direct children of the top level
+        // page at all.
+        if(dynamic_cast<wxStaticBox*>(pw))
+            {
+            ConditionallyEnableChildren(*pw);
+            }
         }
 }
----------------------------------------------------------------------------


 I've done some profiling and the second version is faster but not by as
much as I thought, the difference is ~8%. Considering the time of the
execution of ConditionallyEnable() on my machine is ~500μs, we gain about
40μs from this test which doesn't seem like a big deal, especially
considering that the time of execution of Assimilate() is ~150ms (notice
the units), so I am not sure if it's worth the ugliness of dynamic_cast but
I'm leaving the decision about whether to do it -- and whether to use this
approach at all or try to do something completely different -- to you.


 On a related note, I think it would be really worth it to try to optimize
this code, taking ~0.2s for each update on a fast machine like mine is
clearly unacceptable when the standard maximum acceptable response time is
deemed to be 0.1s. Unfortunately I'm not sure if this can be done by any
micro-optimizations, it looks like the whole approach should be changed by
reducing the number of controls being updated to only the ones which are
really affected by the change. But I could look more into this if you'd
like.

 Best regards,
VZ