added logic to manage automatically allocated ids in-use status to avoid clashes for long-running programs (modified patch 1800016, incidentally fixes bug 1832620)

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@50007 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

include/wx/defs.h

@@ -1772,6 +1772,13 @@ enum wxKeyType
 /*  Standard menu IDs */
 enum
 {
+    /*
+       These ids delimit the range used by automatically-generated ids
+       (i.e. those used when wxID_ANY is specified during construction).
+     */
+    wxID_AUTO_LOWEST = -32000,
+    wxID_AUTO_HIGHEST = -2000,
+
     /* no id matches this one when compared to it */
     wxID_NONE = -3,
 

include/wx/window.h

@@ -201,11 +201,6 @@ public:
     wxWindowVariant GetWindowVariant() const { return m_windowVariant; }
 
 
-        // window id uniquely identifies the window among its siblings unless
-        // it is wxID_ANY which means "don't care"
-    void SetId( wxWindowID winid ) { m_windowId = winid; }
-    wxWindowID GetId() const { return m_windowId; }
-
         // get or change the layout direction (LTR or RTL) for this window,
         // wxLayout_Default is returned if layout direction is not supported
     virtual wxLayoutDirection GetLayoutDirection() const
@@ -219,15 +214,24 @@ public:
                                              wxCoord width,
                                              wxCoord widthTotal) const;
 
-        // generate a control id for the controls which were not given one by
+
-        // user
+        // window id uniquely identifies the window among its siblings unless
-    static int NewControlId() { return --ms_lastControlId; }
+        // it is wxID_ANY which means "don't care"
-        // get the id of the control following the one with the given
+    void SetId( wxWindowID winid ) { m_windowId = winid; }
-        // (autogenerated) id
+    wxWindowID GetId() const { return m_windowId; }
-    static int NextControlId(int winid) { return winid - 1; }
+
-        // get the id of the control preceding the one with the given
+        // returns true if this id value belong to the range reserved for the
-        // (autogenerated) id
+        // auto-generated (by NewControlId()) ids (they're always negative)
-    static int PrevControlId(int winid) { return winid + 1; }
+    static bool IsAutoGeneratedId(wxWindowID id);
+
+        // generate a unique id (or count of them consecutively), returns a
+        // valid id in IsAutoGeneratedId() range or wxID_NONE if failed
+    static wxWindowID NewControlId(int count = 1);
+
+        // mark an id previously returned by NewControlId() as being unused any
+        // more so that it can be reused again for another control later
+    static void ReleaseControlId(wxWindowID id);
+
 
     // moving/resizing
     // ---------------
@@ -1361,6 +1365,10 @@ protected:
     // Layout() window automatically when its size changes?
     bool                 m_autoLayout:1;
 
+    // true if we had automatically allocated the id value for this window
+    // (i.e. wxID_ANY had been passed to the ctor)
+    bool                 m_freeId:1;
+
     // window state
     bool                 m_isShown:1;
     bool                 m_isEnabled:1;
@@ -1528,9 +1536,6 @@ private:
     int DoGetPopupMenuSelectionFromUser(wxMenu& menu, int x, int y);
 #endif // wxUSE_MENUS
 
-    // contains the last id generated by NewControlId
-    static int ms_lastControlId;
-
     // the stack of windows which have captured the mouse
     static struct WXDLLIMPEXP_FWD_CORE wxWindowNext *ms_winCaptureNext;
     // the window that currently has mouse capture

src/common/wincmn.cpp

@@ -89,13 +89,6 @@ WXDLLIMPEXP_DATA_CORE(wxWindowList) wxTopLevelWindows;
 // static data
 // ----------------------------------------------------------------------------
 
-#if defined(__WXPALMOS__)
-int wxWindowBase::ms_lastControlId = 32767;
-#elif defined(__WXPM__)
-int wxWindowBase::ms_lastControlId = 2000;
-#else
-int wxWindowBase::ms_lastControlId = -200;
-#endif
 
 IMPLEMENT_ABSTRACT_CLASS(wxWindowBase, wxEvtHandler)
 
@@ -174,6 +167,7 @@ wxWindowBase::wxWindowBase()
     m_windowSizer = (wxSizer *) NULL;
     m_containingSizer = (wxSizer *) NULL;
     m_autoLayout = false;
+    m_freeId = false;
 
 #if wxUSE_DRAG_AND_DROP
     m_dropTarget = (wxDropTarget *)NULL;
@@ -242,11 +236,22 @@ bool wxWindowBase::CreateBase(wxWindowBase *parent,
     // ids are limited to 16 bits under MSW so if you care about portability,
     // it's not a good idea to use ids out of this range (and negative ids are
     // reserved for wxWidgets own usage)
-    wxASSERT_MSG( id == wxID_ANY || (id >= 0 && id < 32767),
+    wxASSERT_MSG( id == wxID_ANY || (id >= 0 && id < 32767) ||
+                  (id >= wxID_AUTO_LOWEST && id <= wxID_AUTO_HIGHEST),
                   _T("invalid id value") );
 
     // generate a new id if the user doesn't care about it
-    m_windowId = id == wxID_ANY ? NewControlId() : id;
+    if ( id == wxID_ANY )
+    {
+        m_windowId = NewControlId();
+
+        // remember to call ReleaseControlId() when this window is destroyed
+        m_freeId = true;
+    }
+    else // valid id specified
+    {
+        m_windowId = id;
+    }
 
     // don't use SetWindowStyleFlag() here, this function should only be called
     // to change the flag after creation as it tries to reflect the changes in
@@ -302,6 +307,10 @@ wxWindowBase::~wxWindowBase()
 {
     wxASSERT_MSG( GetCapture() != this, wxT("attempt to destroy window with mouse capture") );
 
+    // mark the id as unused if we allocated it for this control
+    if ( m_freeId )
+        ReleaseControlId(m_windowId);
+
     // FIXME if these 2 cases result from programming errors in the user code
     //       we should probably assert here instead of silently fixing them
 
@@ -3180,3 +3189,125 @@ wxWindowBase::AdjustForLayoutDirection(wxCoord x,
     return x;
 }
 
+// ----------------------------------------------------------------------------
+// Window (and menu items) identifiers management
+// ----------------------------------------------------------------------------
+
+namespace
+{
+
+// this array contains, in packed form, the "in use" flags for the entire
+// auto-generated ids range: N-th element of the array contains the flags for
+// ids in [wxID_AUTO_LOWEST + 8*N, wxID_AUTO_LOWEST + 8*N + 7] range
+//
+// initially no ids are in use and we allocate them consecutively, but after we
+// exhaust the entire range, we wrap around and reuse the ids freed in the
+// meanwhile
+wxByte gs_autoIdsInUse[(wxID_AUTO_HIGHEST - wxID_AUTO_LOWEST + 1)/8 + 1] = { 0 };
+
+// this is an optimization used until we wrap around wxID_AUTO_HIGHEST: if this
+// value is < wxID_AUTO_HIGHEST we know that we haven't wrapped yet and so can
+// allocate the ids simply by incrementing it
+static wxWindowID gs_nextControlId = wxID_AUTO_LOWEST;
+
+void MarkAutoIdUsed(wxWindowID id)
+{
+    id -= wxID_AUTO_LOWEST;
+
+    const int theByte = id / 8;
+    const int theBit = id % 8;
+
+    gs_autoIdsInUse[theByte] |= 1 << theBit;
+}
+
+void FreeAutoId(wxWindowID id)
+{
+    id -= wxID_AUTO_LOWEST;
+
+    const int theByte = id / 8;
+    const int theBit = id % 8;
+
+    gs_autoIdsInUse[theByte] &= ~(1 << theBit);
+}
+
+bool IsAutoIdInUse(wxWindowID id)
+{
+    id -= wxID_AUTO_LOWEST;
+
+    const int theByte = id / 8;
+    const int theBit = id % 8;
+
+    return (gs_autoIdsInUse[theByte] & (1 << theBit)) != 0;
+}
+
+} // anonymous namespace
+
+
+/* static */
+bool wxWindowBase::IsAutoGeneratedId(wxWindowID id)
+{
+    if ( id < wxID_AUTO_LOWEST || id > wxID_AUTO_HIGHEST )
+        return false;
+
+    // we shouldn't have any stray ids in this range
+    wxASSERT_MSG( IsAutoIdInUse(id), "unused automatically generated id?" );
+
+    return true;
+}
+
+wxWindowID wxWindowBase::NewControlId(int count)
+{
+    wxASSERT_MSG( count > 0, "can't allocate less than 1 id" );
+
+    if ( gs_nextControlId + count - 1 <= wxID_AUTO_HIGHEST )
+    {
+        // we haven't wrapped yet, so we can just grab the next count ids
+        wxWindowID id = gs_nextControlId;
+
+        while ( count-- )
+            MarkAutoIdUsed(gs_nextControlId++);
+
+        return id;
+    }
+    else // we've already wrapped or are now going to
+    {
+        // brute-force search for the id values
+
+        // number of consecutive free ids found so far
+        int found = 0;
+
+        for ( wxWindowID id = wxID_AUTO_LOWEST; id <= wxID_AUTO_HIGHEST; id++ )
+        {
+            if ( !IsAutoIdInUse(id) )
+            {
+                // found another consecutive available id
+                found++;
+                if ( found == count )
+                {
+                    // mark all count consecutive free ids we found as being in
+                    // use now and rewind back to the start of available range
+                    // in the process
+                    while ( count-- )
+                        MarkAutoIdUsed(id--);
+
+                    return id;
+                }
+            }
+            else // this id is in use
+            {
+                // reset the number of consecutive free values found
+                found = 0;
+            }
+        }
+    }
+
+    // if we get here, there are not enough consecutive free ids
+    return wxID_NONE;
+}
+
+void wxWindowBase::ReleaseControlId(wxWindowID id)
+{
+    wxCHECK_RET( IsAutoGeneratedId(id), "can't release non auto-generated id" );
+
+    FreeAutoId(id);
+}

src/xrc/xmlres.cpp

@@ -47,6 +47,7 @@
 
 #include "wx/xml/xml.h"
 
+
 class wxXmlResourceDataRecord
 {
 public:
@@ -1610,7 +1611,7 @@ static int XRCID_Lookup(const char *str_id, int value_if_not_found = wxID_NONE)
         }
         else
         {
-            (*rec_var)->id = wxWindow::NewControlId();
+            (*rec_var)->id = wxWindowBase::NewControlId();
         }
     }
 
@@ -1639,6 +1640,12 @@ static void CleanXRCID_Record(XRCID_record *rec)
     if (rec)
     {
         CleanXRCID_Record(rec->next);
+
+        // if we had generated the value of this id automatically, release it
+        // now that we don't need it any longer
+        if ( wxWindow::IsAutoGeneratedId(rec->id) )
+            wxWindow::ReleaseControlId(rec->id);
+
         free(rec->key);
         delete rec;
     }