fixed bug with wxcolourDatabase::FindColour(); added (and documented) new Find() and AddColour(const wxColour&) methods to avoid such problems in the future

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

docs/changes.txt

@@ -87,6 +87,7 @@ All (GUI):
 - status text is now restored after wxMenu help is shown in it
 - bug in wxWindow::RemoveEventHandler() fixed (Yingjun Zhang)
 - make it possible to use wxRTTI macros with namespaces (Benjamin I. Williams)
+- wxColourDatabase API now uses objects instead of pointers
 
 wxMSW:
 

docs/latex/wx/colour.tex

@@ -40,6 +40,7 @@ wxLIGHT\_GREY}
 
 \latexignore{\rtfignore{\wxheading{Members}}}
 
+
 \membersection{wxColour::wxColour}\label{wxcolourconstr}
 
 \func{}{wxColour}{\void}
@@ -83,12 +84,14 @@ Copy constructor.
 }
 
 
+
 \membersection{wxColour::Blue}\label{wxcolourblue}
 
 \constfunc{unsigned char}{Blue}{\void}
 
 Returns the blue intensity.
 
+
 \membersection{wxColour::GetPixel}\label{wxcolourgetpixel}
 
 \constfunc{long}{GetPixel}{\void}
@@ -98,17 +101,20 @@ On X, an allocated pixel value is returned.
 
 -1 is returned if the pixel is invalid (on X, unallocated).
 
+
 \membersection{wxColour::Green}\label{wxcolourgreen}
 
 \constfunc{unsigned char}{Green}{\void}
 
 Returns the green intensity.
 
+
 \membersection{wxColour::Ok}\label{wxcolourok}
 
 \constfunc{bool}{Ok}{\void}
 
-Returns true if the colour object is valid (the colour has been initialised with RGB values).
+Returns \true if the colour object is valid (the colour has been initialised with RGB values).
+
 
 \membersection{wxColour::Red}\label{wxcolourred}
 
@@ -116,12 +122,14 @@ Returns true if the colour object is valid (the colour has been initialised with
 
 Returns the red intensity.
 
+
 \membersection{wxColour::Set}\label{wxcolourset}
 
 \func{void}{Set}{\param{const unsigned char}{ red}, \param{const unsigned char}{ green}, \param{const unsigned char}{ blue}}
 
 Sets the RGB intensity values.
 
+
 \membersection{wxColour::operator $=$}\label{wxcolourassign}
 
 \func{wxColour\&}{operator $=$}{\param{const wxColour\&}{ colour}}
@@ -136,12 +144,14 @@ Assignment operator, using a colour name to be found in the colour database.
 
 \helpref{wxColourDatabase}{wxcolourdatabase}
 
+
 \membersection{wxColour::operator $==$}\label{wxcolourequality}
 
 \func{bool}{operator $==$}{\param{const wxColour\&}{ colour}}
 
 Tests the equality of two colours by comparing individual red, green blue colours.
 
+
 \membersection{wxColour::operator $!=$}\label{wxcolourinequality}
 
 \func{bool}{operator $!=$}{\param{const wxColour\&}{ colour}}
@@ -166,6 +176,7 @@ This class holds a variety of information related to colour dialogs.
 
 \latexignore{\rtfignore{\wxheading{Members}}}
 
+
 \membersection{wxColourData::wxColourData}\label{wxcolourdataconstr}
 
 \func{}{wxColourData}{\void}
@@ -173,12 +184,14 @@ This class holds a variety of information related to colour dialogs.
 Constructor. Initializes the custom colours to white, the {\it data colour} setting
 to black, and the {\it choose full} setting to true.
 
+
 \membersection{wxColourData::\destruct{wxColourData}}
 
 \func{}{\destruct{wxColourData}}{\void}
 
 Destructor.
 
+
 \membersection{wxColourData::GetChooseFull}\label{wxcolourdatagetchoosefull}
 
 \constfunc{bool}{GetChooseFull}{\void}
@@ -188,6 +201,7 @@ with custom colour selection controls. Has no meaning under other platforms.
 
 The default value is true.
 
+
 \membersection{wxColourData::GetColour}\label{wxcolourdatagetcolour}
 
 \constfunc{wxColour\&}{GetColour}{\void}
@@ -196,6 +210,7 @@ Gets the current colour associated with the colour dialog.
 
 The default colour is black.
 
+
 \membersection{wxColourData::GetCustomColour}\label{wxcolourdatagetcustomcolour}
 
 \constfunc{wxColour\&}{GetCustomColour}{\param{int}{ i}}
@@ -205,6 +220,7 @@ be an integer between 0 and 15.
 
 The default custom colours are all white.
 
+
 \membersection{wxColourData::SetChooseFull}\label{wxcolourdatasetchoosefull}
 
 \func{void}{SetChooseFull}{\param{const bool }{flag}}
@@ -214,6 +230,7 @@ with custom colour selection controls. Under other platforms, has no effect.
 
 The default value is true.
 
+
 \membersection{wxColourData::SetColour}\label{wxcolourdatasetcolour}
 
 \func{void}{SetColour}{\param{const wxColour\&}{ colour}}
@@ -222,6 +239,7 @@ Sets the default colour for the colour dialog.
 
 The default colour is black.
 
+
 \membersection{wxColourData::SetCustomColour}\label{wxcolourdatasetcustomcolour}
 
 \func{void}{SetCustomColour}{\param{int}{ i}, \param{const wxColour\&}{ colour}}
@@ -231,18 +249,27 @@ be an integer between 0 and 15.
 
 The default custom colours are all white.
 
+
 \membersection{wxColourData::operator $=$}\label{wxcolourdataassign}
 
 \func{void}{operator $=$}{\param{const wxColourData\&}{ data}}
 
 Assignment operator for the colour data.
 
+
+
+
 \section{\class{wxColourDatabase}}\label{wxcolourdatabase}
 
 wxWindows maintains a database of standard RGB colours for a predefined
 set of named colours (such as ``BLACK'', ``LIGHT GREY''). The
-application may add to this set if desired by using {\it Append}.  There
+application may add to this set if desired by using 
-is only one instance of this class: {\bf wxTheColourDatabase}.
+\helpref{AddColour}{wxcolourdatabaseaddcolour} and may use it to look up
+colours by names using \helpref{Find}{wxcolourdatabasefind} or find the names
+for the standard colour suing \helpref{FindName}{wxcolourdatabasefindname}.
+
+There is one predefined instance of this class called 
+{\bf wxTheColourDatabase}.
 
 \wxheading{Derived from}
 
@@ -254,7 +281,7 @@ None
 
 \wxheading{Remarks}
 
-The colours in the standard database are as follows:
+The standard database contains at least the following colours:
 
 AQUAMARINE, BLACK, BLUE, BLUE VIOLET, BROWN, CADET BLUE, CORAL,
 CORNFLOWER BLUE, CYAN, DARK GREY, DARK GREEN, DARK OLIVE GREEN, DARK
@@ -275,36 +302,53 @@ YELLOW GREEN.
 
 \latexignore{\rtfignore{\wxheading{Members}}}
 
+
 \membersection{wxColourDatabase::wxColourDatabase}\label{wxcolourdatabaseconstr}
 
 \func{}{wxColourDatabase}{\void}
 
-Constructs the colour database.
+Constructs the colour database. It will be initialized at the first use.
+
 
 \membersection{wxColourDatabase::AddColour}\label{wxcolourdatabaseaddcolour}
 
+\func{void}{AddColour}{\param{const wxString\& }{colourName}, \param{const wxColour\&}{colour}}
+
 \func{void}{AddColour}{\param{const wxString\& }{colourName}, \param{wxColour* }{colour}}
 
 Adds a colour to the database. If a colour with the same name already exists,
 it is replaced.
 
+Please note that the overload taking a pointer is deprecated and will be
+removed in the next wxWindows version, please don't use it.
+
+
+\membersection{wxColourDatabase::Find}\label{wxcolourdatabasefind}
+
+\func{wxColour}{FindColour}{\param{const wxString\& }{colourName}}
+
+Finds a colour given the name. Returns an invalid colour object (that is, such
+that its \helpref{Ok()}{wxcolourok} method returns \false) if the colour wasn't
+found in the database.
+
+
 \membersection{wxColourDatabase::FindColour}\label{wxcolourdatabasefindcolour}
 
 \func{wxColour*}{FindColour}{\param{const wxString\& }{colourName}}
 
-Finds a colour given the name. Returns NULL if not found.
+Finds a colour given the name. Returns \NULL if not found or a pointer which
+must be deleted by the caller otherwise.
+
+Please note that this method is deprecated and will be removed in the next
+wxWindows version, please use \helpref{Find}{wxcolourdatabasefind} instead of
+it.
+
 
 \membersection{wxColourDatabase::FindName}\label{wxcolourdatabasefindname}
 
 \constfunc{wxString}{FindName}{\param{const wxColour\&}{ colour}}
 
-Finds a colour name given the colour. Returns NULL if not found.
+Finds a colour name given the colour. Returns an empty string if the colour is
-
+not found in the database.
-\membersection{wxColourDatabase::Initialize}\label{wxcolourdatabaseinitialize}
-
-\func{void}{Initialize}{\void}
-
-Initializes the database with a number of stock colours.  Called by wxWindows
-on start-up.
 
 

include/wx/gdicmn.h

@@ -417,23 +417,32 @@ class WXDLLEXPORT wxColourDatabase
 {
 public:
     wxColourDatabase();
-    ~wxColourDatabase() ;
+    ~wxColourDatabase();
 
-    // Not const because it may add a name to the database
+    // find colour by name or name for the given colour
-    wxColour *FindColour(const wxString& colour) ;
+    wxColour Find(const wxString& name) const;
-    wxColour *FindColourNoAdd(const wxString& colour) const;
     wxString FindName(const wxColour& colour) const;
-    void AddColour(const wxString& name, wxColour* colour);
+
-    void Initialize();
+    // add a new colour to the database
+    void AddColour(const wxString& name, const wxColour& colour);
+
+    // deprecated, use Find()/Add() instead
+    wxColour *FindColour(const wxString& name);
+    void AddColour(const wxString& name, wxColour *colour);
+
+
 #ifdef __WXPM__
     // PM keeps its own type of colour table
     long*                           m_palTable;
     size_t                          m_nSize;
 #endif
-private:
-    wxColour* FindColour(const wxString& colour, bool add);
 
-    wxStringToColourHashMap* m_map;
+private:
+    // load the database with the built in colour values when called for the
+    // first time, do nothing after this
+    void Initialize();
+
+    wxStringToColourHashMap *m_map;
 };
 
 class WXDLLEXPORT wxBitmapList : public wxList

src/common/gdicmn.cpp

@@ -213,27 +213,47 @@ bool wxRect::Intersects(const wxRect& rect) const
     return r.width != 0;
 }
 
-WX_DECLARE_STRING_HASH_MAP( wxColour*, wxStringToColourHashMap );
+// ============================================================================
+// wxColourDatabase
+// ============================================================================
+
+WX_DECLARE_STRING_HASH_MAP( wxColour *, wxStringToColourHashMap );
+
+// ----------------------------------------------------------------------------
+// wxColourDatabase ctor/dtor
+// ----------------------------------------------------------------------------
 
 wxColourDatabase::wxColourDatabase ()
 {
-    m_map = new wxStringToColourHashMap;
+    // will be created on demand in Initialize()
+    m_map = NULL;
 }
 
 wxColourDatabase::~wxColourDatabase ()
 {
-    WX_CLEAR_HASH_MAP(wxStringToColourHashMap, *m_map);
+    if ( m_map )
+    {
+        WX_CLEAR_HASH_MAP(wxStringToColourHashMap, *m_map);
+
+        delete m_map;
+    }
 
-    delete m_map;
-    m_map = NULL;
 #ifdef __WXPM__
     delete [] m_palTable;
 #endif
 }
 
 // Colour database stuff
-void wxColourDatabase::Initialize ()
+void wxColourDatabase::Initialize()
 {
+    if ( m_map )
+    {
+        // already initialized
+        return;
+    }
+
+    m_map = new wxStringToColourHashMap;
+
     static const struct wxColourDesc
     {
         const wxChar *name;
@@ -315,13 +335,14 @@ void wxColourDatabase::Initialize ()
         {wxT("YELLOW GREEN"), 153, 204, 50}
     };
 
-    size_t      n;
+    size_t n;
 
     for ( n = 0; n < WXSIZEOF(wxColourTable); n++ )
     {
         const wxColourDesc& cc = wxColourTable[n];
-        AddColour(cc.name, new wxColour(cc.r,cc.g,cc.b));
+        (*m_map)[cc.name] = new wxColour(cc.r, cc.g, cc.b);
     }
+
 #ifdef __WXPM__
     m_palTable = new long[n];
     for ( n = 0; n < WXSIZEOF(wxColourTable); n++ )
@@ -333,99 +354,74 @@ void wxColourDatabase::Initialize ()
 #endif
 }
 
-/*
+// ----------------------------------------------------------------------------
- * Changed by Ian Brown, July 1994.
+// wxColourDatabase operations
- *
+// ----------------------------------------------------------------------------
- * When running under X, the Colour Database starts off empty. The X server
- * is queried for the colour first time after which it is entered into the
- * database. This allows our client to use the server colour database which
- * is hopefully gamma corrected for the display being used.
- */
 
-wxColour *wxColourDatabase::FindColour(const wxString& colour)
+void wxColourDatabase::AddColour(const wxString& name, const wxColour& colour)
 {
-    return FindColour(colour, true);
+    Initialize();
-}
 
-wxColour *wxColourDatabase::FindColourNoAdd(const wxString& colour) const
+    // canonicalize the colour names before using them as keys: they should be
-{
+    // in upper case
-    return ((wxColourDatabase*)this)->FindColour(colour, false);
-}
-
-void wxColourDatabase::AddColour (const wxString& name, wxColour* colour)
-{
     wxString colName = name;
     colName.MakeUpper();
-    wxString colName2 = colName;
-    if ( !colName2.Replace(_T("GRAY"), _T("GREY")) )
-        colName2.clear();
 
-    wxStringToColourHashMap::iterator it = m_map->find(colName);
+    // ... and we also allow both grey/gray
-    if ( it == m_map->end() )
+    wxString colNameAlt = colName;
-        it = m_map->find(colName2);
+    if ( !colNameAlt.Replace(_T("GRAY"), _T("GREY")) )
-    if ( it != m_map->end() )
     {
-        delete it->second;
+        // but in this case it is not necessary so avoid extra search below
-        it->second = colour;
+        colNameAlt.clear();
     }
 
-    (*m_map)[name] = colour;
+    wxStringToColourHashMap::iterator it = m_map->find(colName);
+    if ( it == m_map->end() && !colNameAlt.empty() )
+        it = m_map->find(colNameAlt);
+    if ( it != m_map->end() )
+    {
+        *(it->second) = colour;
+    }
+    else // new colour
+    {
+        (*m_map)[name] = new wxColour(colour);
+    }
 }
 
-wxColour *wxColourDatabase::FindColour(const wxString& colour, bool add)
+wxColour wxColourDatabase::Find(const wxString& colour) const
 {
-    // VZ: make the comparaison case insensitive and also match both grey and
+    wxColourDatabase * const self = wxConstCast(this, wxColourDatabase);
-    //     gray
+    self->Initialize();
+
+    // first look among the existing colours
+
+    // make the comparaison case insensitive and also match both grey and gray
     wxString colName = colour;
     colName.MakeUpper();
-    wxString colName2 = colName;
+    wxString colNameAlt = colName;
-    if ( !colName2.Replace(_T("GRAY"), _T("GREY")) )
+    if ( !colNameAlt.Replace(_T("GRAY"), _T("GREY")) )
-        colName2.clear();
+        colNameAlt.clear();
 
     wxStringToColourHashMap::iterator it = m_map->find(colName);
-    if ( it == m_map->end() )
+    if ( it == m_map->end() && !colNameAlt.empty() )
-        it = m_map->find(colName2);
+        it = m_map->find(colNameAlt);
     if ( it != m_map->end() )
-        return it->second;
+        return *(it->second);
-
-    if ( !add )
-        return NULL;
-
-#ifdef __WXMSW__
-  return NULL;
-#endif
-#ifdef __WXPM__
-  return NULL;
-#endif
-#ifdef __WXMGL__
-  return NULL;
-#endif
-
-// TODO for other implementations. This should really go into
-// platform-specific directories.
-#ifdef __WXMAC__
-  return NULL;
-#endif
-#ifdef __WXCOCOA__
-  return NULL;
-#endif
-#ifdef __WXSTUBS__
-  return NULL;
-#endif
 
+    // if we didn't find it,query the system, maybe it knows about it
+    //
+    // TODO: move this into platform-specific files
 #ifdef __WXGTK__
-  wxColour *col = new wxColour( colour );
+    wxColour col( colour );
 
-  if (!(col->Ok()))
+    if ( col.Ok() )
-  {
+    {
-      delete col;
+        // cache it
-      return (wxColour *) NULL;
+        self->AddColour(colour, col);
-  }
+    }
-  AddColour(colour, col);
-  return col;
-#endif
 
-#ifdef __X__
+    return col;
+#elif defined(__X__)
     XColor xcolour;
 
 #ifdef __WXMOTIF__
@@ -436,7 +432,7 @@ wxColour *wxColourDatabase::FindColour(const wxString& colour, bool add)
 #endif
     /* MATTHEW: [4] Use wxGetMainColormap */
     if (!XParseColor(display, (Colormap) wxTheApp->GetMainColormap((WXDisplay*) display), colour.ToAscii() ,&xcolour))
-      return NULL;
+        return NULL;
 
 #if wxUSE_NANOX
     unsigned char r = (unsigned char)(xcolour.red);
@@ -448,36 +444,58 @@ wxColour *wxColourDatabase::FindColour(const wxString& colour, bool add)
     unsigned char b = (unsigned char)(xcolour.blue >> 8);
 #endif
 
-    wxColour *col = new wxColour(r, g, b);
+    wxColour col(r, g, b);
     AddColour(colour, col);
 
     return col;
-#endif // __X__
+#else // other platform
+    return wxNullColour;
+#endif // platforms
 }
 
-wxString wxColourDatabase::FindName (const wxColour& colour) const
+wxString wxColourDatabase::FindName(const wxColour& colour) const
 {
-    unsigned char red = colour.Red ();
+    wxColourDatabase * const self = wxConstCast(this, wxColourDatabase);
-    unsigned char green = colour.Green ();
+    self->Initialize();
-    unsigned char blue = colour.Blue ();
 
     typedef wxStringToColourHashMap::iterator iterator;
 
-    for (iterator it = m_map->begin(), en = m_map->end(); it != en; ++it )
+    for ( iterator it = m_map->begin(), en = m_map->end(); it != en; ++it )
     {
-        wxColour *col = it->second;
+        if ( *(it->second) == colour )
-
-        if (col->Red () == red && col->Green () == green && col->Blue () == blue)
             return it->first;
     }
 
     return wxEmptyString;
 }
 
+// ----------------------------------------------------------------------------
+// deprecated wxColourDatabase methods
+// ----------------------------------------------------------------------------
+
+wxColour *wxColourDatabase::FindColour(const wxString& name)
+{
+    wxColour col = Find(name);
+    if ( !col.Ok() )
+        return NULL;
+
+    return new wxColour(col);
+}
+
+void wxColourDatabase::AddColour(const wxString& name, wxColour *colour)
+{
+    wxCHECK_RET( colour, _T("NULL pointer in wxColourDatabase::AddColour") );
+
+    AddColour(name, wxColour(*colour));
+}
+
+// ============================================================================
+// stock objects
+// ============================================================================
+
 void wxInitializeStockLists()
 {
     wxTheColourDatabase = new wxColourDatabase;
-    wxTheColourDatabase->Initialize();
 
     wxTheBrushList = new wxBrushList;
     wxThePenList = new wxPenList;