Merge branch 'dvc-type-check'

Various fixes to make wxDataViewCtrl behaviour more homogeneous across all
ports.

include/wx/dvrenderers.h

@@ -115,15 +115,21 @@ public:
     virtual bool SetValue(const wxVariant& value) = 0;
     virtual bool GetValue(wxVariant& value) const = 0;
 
-    virtual void SetAttr(const wxDataViewItemAttr& WXUNUSED(attr)) { }
-
-    virtual void SetEnabled(bool WXUNUSED(enabled)) { }
-
     wxString GetVariantType() const             { return m_variantType; }
 
-    // helper that calls SetValue and SetAttr:
+    // Prepare for rendering the value of the corresponding item in the given
-    void PrepareForItem(const wxDataViewModel *model,
+    // column taken from the provided non-null model.
-                        const wxDataViewItem& item, unsigned column);
+    //
+    // Notice that the column must be the same as GetOwner()->GetModelColumn(),
+    // it is only passed to this method because the existing code already has
+    // it and should probably be removed in the future.
+    //
+    // Return true if this cell is non-empty or false otherwise (and also if
+    // the model returned a value of the wrong, i.e. different from our
+    // GetVariantType(), type, in which case a debug error is also logged).
+    bool PrepareForItem(const wxDataViewModel *model,
+                        const wxDataViewItem& item,
+                        unsigned column);
 
     // renderer properties:
     virtual void SetMode( wxDataViewCellMode mode ) = 0;
@@ -175,9 +181,21 @@ public:
     int GetEffectiveAlignment() const;
 
 protected:
+    // These methods are called from PrepareForItem() and should do whatever is
+    // needed for the current platform to ensure that the item is rendered
+    // using the given attributes and enabled/disabled state.
+    virtual void SetAttr(const wxDataViewItemAttr& attr) = 0;
+    virtual void SetEnabled(bool enabled) = 0;
+
     // Called from {Cancel,Finish}Editing() to cleanup m_editorCtrl
     void DestroyEditControl();
 
+    // Helper of PrepareForItem() also used in StartEditing(): returns the
+    // value checking that its type matches our GetVariantType().
+    wxVariant CheckedGetValue(const wxDataViewModel* model,
+                              const wxDataViewItem& item,
+                              unsigned column) const;
+
     wxString                m_variantType;
     wxDataViewColumn       *m_owner;
     wxWeakRef<wxWindow>     m_editorCtrl;
@@ -292,7 +310,7 @@ public:
 
     // Store the enabled state of the item so that it can be accessed from
     // Render() via GetEnabled() if needed.
-    virtual void SetEnabled(bool enabled) { m_enabled = enabled; }
+    virtual void SetEnabled(bool enabled);
     bool GetEnabled() const { return m_enabled; }
 
 

include/wx/gtk/dvrenderer.h

@@ -55,37 +55,21 @@ public:
     void GtkInitHandlers();
     void GtkUpdateAlignment() { GtkApplyAlignment(m_renderer); }
 
-    // should be overridden to return true if the renderer supports properties
-    // corresponding to wxDataViewItemAttr field, see wxGtkTreeCellDataFunc()
-    // for details
-    virtual bool GtkSupportsAttrs() const { return false; }
-
-    // if GtkSupportsAttrs() returns true, this function will be called to
-    // effectively set the attribute to use for rendering the next item
-    //
-    // it should return true if the attribute had any non-default properties
-    virtual bool GtkSetAttr(const wxDataViewItemAttr& WXUNUSED(attr))
-        { return false; }
-
-
-    // these functions are only called if GtkSupportsAttrs() returns true and
-    // are used to remember whether the renderer currently uses the default
-    // attributes or if we changed (and not reset them)
-    bool GtkIsUsingDefaultAttrs() const { return m_usingDefaultAttrs; }
-    void GtkSetUsingDefaultAttrs(bool def) { m_usingDefaultAttrs = def; }
-
     // return the text renderer used by this renderer for setting text cell
     // specific attributes: can return NULL if this renderer doesn't render any
     // text
     virtual GtkCellRendererText *GtkGetTextRenderer() const { return NULL; }
 
-
+private:
     // Change the mode at GTK level without touching m_mode, this is useful for
     // temporarily making the renderer insensitive but does mean that GetMode()
     // may return a value different from the actual GTK renderer mode.
     void GtkSetMode(wxDataViewCellMode mode);
 
 protected:
+    virtual void SetAttr(const wxDataViewItemAttr& attr) wxOVERRIDE;
+    virtual void SetEnabled(bool enabled) wxOVERRIDE;
+
     virtual void GtkOnCellChanged(const wxVariant& value,
                                   const wxDataViewItem& item,
                                   unsigned col);

include/wx/gtk/dvrenderers.h

@@ -49,12 +49,11 @@ public:
 
     virtual void SetAlignment( int align );
 
-    virtual bool GtkSupportsAttrs() const { return true; }
-    virtual bool GtkSetAttr(const wxDataViewItemAttr& attr);
-
     virtual GtkCellRendererText *GtkGetTextRenderer() const;
 
 protected:
+    virtual void SetAttr(const wxDataViewItemAttr& attr) wxOVERRIDE;
+
     // implementation of Set/GetValue()
     bool SetTextValue(const wxString& str);
     bool GetTextValue(wxString& str) const;
@@ -137,16 +136,6 @@ public:
         m_renderParams = renderParams;
     }
 
-    // we may or not support attributes, as we don't know it, return true to
-    // make it possible to use them
-    virtual bool GtkSupportsAttrs() const { return true; }
-
-    virtual bool GtkSetAttr(const wxDataViewItemAttr& attr)
-    {
-        SetAttr(attr);
-        return !attr.IsDefault();
-    }
-
     virtual GtkCellRendererText *GtkGetTextRenderer() const;
 
 private:

include/wx/osx/dvrenderer.h

@@ -83,12 +83,9 @@ public:
                                   const wxDataViewItem& item,
                                   unsigned col);
 
-    // called to ensure that the given attribute will be used for rendering the
+protected:
-    // next cell (which had been already associated with this renderer before)
+    virtual void SetAttr(const wxDataViewItemAttr& attr) wxOVERRIDE;
-    virtual void OSXApplyAttr(const wxDataViewItemAttr& attr);
+    virtual void SetEnabled(bool enabled) wxOVERRIDE;
-
-    // called to set the state of the next cell to be rendered
-    virtual void OSXApplyEnabled(bool enabled);
 #endif // Cocoa
 
 private:

include/wx/osx/dvrenderers.h

@@ -31,10 +31,6 @@ public:
 
     virtual bool MacRender();
 
-#if wxOSX_USE_COCOA
-    virtual void OSXApplyAttr(const wxDataViewItemAttr& attr);
-#endif // Cocoa
-
     virtual wxDC* GetDC(); // creates a device context and keeps it
     void SetDC(wxDC* newDCPtr); // this method takes ownership of the pointer
 

src/common/datavcmn.cpp

@@ -683,8 +683,7 @@ bool wxDataViewRendererBase::StartEditing( const wxDataViewItem &item, wxRect la
     m_item = item; // remember for later
 
     unsigned int col = GetOwner()->GetModelColumn();
-    wxVariant value;
+    const wxVariant& value = CheckedGetValue(dv_ctrl->GetModel(), item, col);
-    dv_ctrl->GetModel()->GetValue( value, item, col );
 
     m_editorCtrl = CreateEditorCtrl( dv_ctrl->GetMainWindow(), labelRect, value );
 
@@ -778,19 +777,71 @@ bool wxDataViewRendererBase::FinishEditing()
     return false;
 }
 
-void wxDataViewRendererBase::PrepareForItem(const wxDataViewModel *model,
+wxVariant
+wxDataViewRendererBase::CheckedGetValue(const wxDataViewModel* model,
                                         const wxDataViewItem& item,
-                                            unsigned column)
+                                        unsigned column) const
 {
     wxVariant value;
     model->GetValue(value, item, column);
+
+    // We always allow the cell to be null, regardless of the renderer type.
+    if ( !value.IsNull() )
+    {
+        if ( value.GetType() != GetVariantType() )
+        {
+            // If you're seeing this message, this indicates that either your
+            // renderer is using the wrong type, or your model returns values
+            // of the wrong type.
+            wxLogDebug("Wrong type returned from the model for column %u: "
+                       "%s required but actual type is %s",
+                       column,
+                       GetVariantType(),
+                       value.GetType());
+
+            // Don't return data of mismatching type, this could be unexpected.
+            value.MakeNull();
+        }
+    }
+
+    return value;
+}
+
+bool
+wxDataViewRendererBase::PrepareForItem(const wxDataViewModel *model,
+                                       const wxDataViewItem& item,
+                                       unsigned column)
+{
+    // Now check if we have a value and remember it if we do.
+    const wxVariant& value = CheckedGetValue(model, item, column);
+    if ( !value.IsNull() )
+    {
         SetValue(value);
 
+        // Also set up the attributes for this item if it's not empty.
         wxDataViewItemAttr attr;
         model->GetAttr(item, column, attr);
         SetAttr(attr);
+    }
 
-    SetEnabled(model->IsEnabled(item, column));
+    // Finally determine the enabled/disabled state and apply it, even to the
+    // empty cells.
+    bool enabled = true;
+    switch ( GetMode() )
+    {
+        case wxDATAVIEW_CELL_INERT:
+            enabled = false;
+            break;
+
+        case wxDATAVIEW_CELL_ACTIVATABLE:
+        case wxDATAVIEW_CELL_EDITABLE:
+            enabled = model->IsEnabled(item, column);
+            break;
+    }
+
+    SetEnabled(enabled);
+
+    return true;
 }
 
 
@@ -948,6 +999,18 @@ wxDataViewCustomRendererBase::RenderText(const wxString& text,
                   rectText, GetEffectiveAlignment());
 }
 
+void wxDataViewCustomRendererBase::SetEnabled(bool enabled)
+{
+    // The native base renderer needs to know about the enabled state as well
+    // but in the generic case the base class method is pure, so we can't just
+    // call it unconditionally.
+#ifndef wxHAS_GENERIC_DATAVIEWCTRL
+    wxDataViewRenderer::SetEnabled(enabled);
+#endif // !wxHAS_GENERIC_DATAVIEWCTRL
+
+    m_enabled = enabled;
+}
+
 //-----------------------------------------------------------------------------
 // wxDataViewEditorCtrlEvtHandler
 //-----------------------------------------------------------------------------

src/gtk/dataview.cpp

@@ -1889,6 +1889,24 @@ void wxDataViewRenderer::SetMode( wxDataViewCellMode mode )
     GtkSetMode(mode);
 }
 
+void wxDataViewRenderer::SetEnabled(bool enabled)
+{
+    // a) this sets the appearance to disabled grey and should only be done for
+    // the active cells which are disabled, not for the cells which can never
+    // be edited at all
+    if ( GetMode() != wxDATAVIEW_CELL_INERT )
+    {
+        GValue gvalue = G_VALUE_INIT;
+        g_value_init( &gvalue, G_TYPE_BOOLEAN );
+        g_value_set_boolean( &gvalue, enabled );
+        g_object_set_property( G_OBJECT(m_renderer), "sensitive", &gvalue );
+        g_value_unset( &gvalue );
+    }
+
+    // b) this actually disables the control/renderer
+    GtkSetMode(enabled ? GetMode() : wxDATAVIEW_CELL_INERT);
+}
+
 void wxDataViewRenderer::GtkSetMode( wxDataViewCellMode mode )
 {
     GtkCellRendererMode gtkMode;
@@ -2031,6 +2049,12 @@ wxDataViewRenderer::GtkOnCellChanged(const wxVariant& value,
     model->ChangeValue( value, item, col );
 }
 
+void wxDataViewRenderer::SetAttr(const wxDataViewItemAttr& WXUNUSED(attr))
+{
+    // There is no way to apply attributes to an arbitrary renderer, so we
+    // simply can't do anything here.
+}
+
 // ---------------------------------------------------------
 // wxDataViewTextRenderer
 // ---------------------------------------------------------
@@ -2054,10 +2078,9 @@ namespace
 
 // helper function used by wxDataViewTextRenderer and
 // wxDataViewCustomRenderer::RenderText(): it applies the attributes to the
-// given text renderer and returns true if anything was done
+// given text renderer
-bool GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
+void GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
 {
-    bool usingDefaultAttrs = true;
     if (attr.HasColour())
     {
         const GdkColor * const gcol = attr.GetColour().GetColor();
@@ -2067,8 +2090,6 @@ bool GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
         g_value_set_boxed( &gvalue, gcol );
         g_object_set_property( G_OBJECT(renderer), "foreground_gdk", &gvalue );
         g_value_unset( &gvalue );
-
-        usingDefaultAttrs = false;
     }
     else
     {
@@ -2086,8 +2107,6 @@ bool GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
         g_value_set_enum( &gvalue, PANGO_STYLE_ITALIC );
         g_object_set_property( G_OBJECT(renderer), "style", &gvalue );
         g_value_unset( &gvalue );
-
-        usingDefaultAttrs = false;
     }
     else
     {
@@ -2106,8 +2125,6 @@ bool GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
         g_value_set_enum( &gvalue, PANGO_WEIGHT_BOLD );
         g_object_set_property( G_OBJECT(renderer), "weight", &gvalue );
         g_value_unset( &gvalue );
-
-        usingDefaultAttrs = false;
     }
     else
     {
@@ -2139,8 +2156,6 @@ bool GtkApplyAttr(GtkCellRendererText *renderer, const wxDataViewItemAttr& attr)
         g_value_unset( &gvalue );
     }
 #endif
-
-    return !usingDefaultAttrs;
 }
 
 } // anonymous namespace
@@ -2217,9 +2232,16 @@ void wxDataViewTextRenderer::SetAlignment( int align )
     g_value_unset( &gvalue );
 }
 
-bool wxDataViewTextRenderer::GtkSetAttr(const wxDataViewItemAttr& attr)
+void wxDataViewTextRenderer::SetAttr(const wxDataViewItemAttr& attr)
 {
-    return GtkApplyAttr(GtkGetTextRenderer(), attr);
+    // An optimization: don't bother resetting the attributes if we're already
+    // using the defaults.
+    if ( attr.IsDefault() && m_usingDefaultAttrs )
+        return;
+
+    GtkApplyAttr(GtkGetTextRenderer(), attr);
+
+    m_usingDefaultAttrs = attr.IsDefault();
 }
 
 GtkCellRendererText *wxDataViewTextRenderer::GtkGetTextRenderer() const
@@ -2890,6 +2912,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column),
     wxDataViewRenderer *cell = (wxDataViewRenderer*) data;
 
     wxDataViewItem item( (void*) iter->user_data );
+    const unsigned column = cell->GetOwner()->GetModelColumn();
 
     wxDataViewModel *wx_model = tree_model->internal->GetDataViewModel();
 
@@ -2898,8 +2921,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column),
         gboolean visible;
         if (wx_model->IsContainer( item ))
         {
-            visible = wx_model->HasContainerColumns( item ) ||
+            visible = wx_model->HasContainerColumns( item ) || (column == 0);
-                        (cell->GetOwner()->GetModelColumn() == 0);
         }
         else
         {
@@ -2916,53 +2938,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column),
             return;
     }
 
-    wxVariant value;
+    cell->PrepareForItem(wx_model, item, column);
-    wx_model->GetValue( value, item, cell->GetOwner()->GetModelColumn() );
-
-    // It is always possible to leave a cell empty, don't warn about type
-    // mismatch in this case.
-    if (!value.IsNull())
-    {
-        if (value.GetType() != cell->GetVariantType())
-        {
-            wxLogDebug("Wrong type returned from the model: "
-                       "%s required but actual type is %s",
-                       cell->GetVariantType(),
-                       value.GetType());
-        }
-    }
-
-    cell->SetValue( value );
-
-    // deal with disabled items
-    bool enabled = wx_model->IsEnabled( item, cell->GetOwner()->GetModelColumn() );
-
-    // a) this sets the appearance to disabled grey    
-    GValue gvalue = G_VALUE_INIT;
-    g_value_init( &gvalue, G_TYPE_BOOLEAN );
-    g_value_set_boolean( &gvalue, enabled );
-    g_object_set_property( G_OBJECT(renderer), "sensitive", &gvalue );
-    g_value_unset( &gvalue );
-
-    // b) this actually disables the control/renderer
-    cell->GtkSetMode(enabled ? cell->GetMode() : wxDATAVIEW_CELL_INERT);
-
-    // deal with attributes: if the renderer doesn't support them at all, we
-    // don't even need to query the model for them
-    if ( !cell->GtkSupportsAttrs() )
-        return;
-
-    // it can support attributes so check if this item has any
-    wxDataViewItemAttr attr;
-    if ( wx_model->GetAttr( item, cell->GetOwner()->GetModelColumn(), attr )
-            || !cell->GtkIsUsingDefaultAttrs() )
-    {
-        bool usingDefaultAttrs = !cell->GtkSetAttr(attr);
-        cell->GtkSetUsingDefaultAttrs(usingDefaultAttrs);
-    }
-    // else: no custom attributes specified and we're already using the default
-    //       ones -- nothing to do
-    
 }
 
 } // extern "C"

src/osx/carbon/dataview.cpp

@@ -1410,7 +1410,6 @@ OSStatus wxMacDataViewDataBrowserListViewControl::DataBrowserGetSetItemDataProc(
     if (propertyID >= kMinPropertyID) // in case data columns set the data
     {
      // variable definitions:
-      wxVariant         variant;
       wxDataViewColumn* dataViewColumnPtr;
       wxDataViewCtrl*   dataViewCtrlPtr;
 
@@ -1420,11 +1419,9 @@ OSStatus wxMacDataViewDataBrowserListViewControl::DataBrowserGetSetItemDataProc(
       dataViewColumnPtr = GetColumnPtr(propertyID);
       wxCHECK_MSG(dataViewColumnPtr                != NULL,errDataBrowserNotConfigured,_("No column for the specified column position existing."));
       wxCHECK_MSG(dataViewColumnPtr->GetRenderer() != NULL,errDataBrowserNotConfigured,_("No renderer specified for column."));
-      dataViewCtrlPtr->GetModel()->GetValue(variant,wxDataViewItem(reinterpret_cast<void*>(itemID)),dataViewColumnPtr->GetModelColumn());
+      if (dataViewColumnPtr->GetRenderer()->PrepareForItem(dataViewCtrlPtr->GetModel(),wxDataViewItem(reinterpret_cast<void*>(itemID)),dataViewColumnPtr->GetModelColumn()))
-      if (!(variant.IsNull()))
       {
         dataViewColumnPtr->GetRenderer()->GetNativeData()->SetItemDataRef(itemData);
-        dataViewColumnPtr->GetRenderer()->SetValue(variant);
         wxCHECK_MSG(dataViewColumnPtr->GetRenderer()->MacRender(),errDataBrowserNotConfigured,_("Rendering failed."));
       }
       return noErr;

src/osx/cocoa/dataview.mm

@@ -1796,53 +1796,12 @@ outlineView:(NSOutlineView*)outlineView
     data->SetItem(item);
     data->SetItemCell(cell);
 
-    // set the state (enabled/disabled) of the item: this must be done first as
-    // even if we return below because the cell is empty, it still needs to be
-    // disabled if it's not supposed to be enabled
-    bool enabled = true;
-    switch ( renderer->GetMode() )
-    {
-        case wxDATAVIEW_CELL_INERT:
-            enabled = false;
-            break;
-
-        case wxDATAVIEW_CELL_ACTIVATABLE:
-        case wxDATAVIEW_CELL_EDITABLE:
-            enabled = model->IsEnabled(dvItem, colIdx);
-            break;
-    }
-    renderer->OSXApplyEnabled(enabled);
-
     // check if we have anything to render
-    wxVariant value;
+    if ( renderer->PrepareForItem(model, dvItem, colIdx) )
-    model->GetValue(value, dvItem, colIdx);
-    if ( value.IsNull() )
     {
-        // for consistency with the generic implementation, just handle missing
+        // and do render it in this case
-        // values as blank
-        return;
-    }
-
-    if ( value.GetType() != renderer->GetVariantType() )
-    {
-        wxLogDebug("Wrong type returned from the model: "
-                   "%s required but actual type is %s",
-                   renderer->GetVariantType(),
-                   value.GetType());
-
-        // we can't use the value of wrong type
-        return;
-    }
-
-    // use the attributes: notice that we need to do this whether we have them
-    // or not as even if this cell doesn't have any attributes, the previous
-    // one might have had some and then we need to reset them back to default
-    wxDataViewItemAttr attr;
-    model->GetAttr(dvItem, colIdx, attr);
-    renderer->OSXApplyAttr(attr);
-
-    // and finally do draw it
         renderer->MacRender();
+    }
 }
 
 //
@@ -2721,7 +2680,7 @@ wxDataViewRenderer::OSXOnCellChanged(NSObject *object,
     model->ChangeValue(value, item, col);
 }
 
-void wxDataViewRenderer::OSXApplyAttr(const wxDataViewItemAttr& attr)
+void wxDataViewRenderer::SetAttr(const wxDataViewItemAttr& attr)
 {
     wxDataViewRendererNativeData * const data = GetNativeData();
     NSCell * const cell = data->GetItemCell();
@@ -2790,7 +2749,7 @@ void wxDataViewRenderer::OSXApplyAttr(const wxDataViewItemAttr& attr)
         [(id)cell setTextColor:colText];
 }
 
-void wxDataViewRenderer::OSXApplyEnabled(bool enabled)
+void wxDataViewRenderer::SetEnabled(bool enabled)
 {
     [GetNativeData()->GetItemCell() setEnabled:enabled];
 }
@@ -2818,18 +2777,6 @@ bool wxDataViewCustomRenderer::MacRender()
     return true;
 }
 
-void wxDataViewCustomRenderer::OSXApplyAttr(const wxDataViewItemAttr& attr)
-{
-    // simply save the attribute so that it could be reused from our Render()
-    SetAttr(attr);
-
-    // it's not necessary to call the base class version which sets the cell
-    // properties to correspond to this attribute because we currently don't
-    // use any NSCell methods in custom renderers anyhow but if we ever start
-    // doing this (e.g. override RenderText() here to use NSTextFieldCell
-    // methods), then we should pass it on to wxDataViewRenderer here
-}
-
 wxIMPLEMENT_ABSTRACT_CLASS(wxDataViewCustomRenderer, wxDataViewRenderer);
 
 // ---------------------------------------------------------