Use wxDataViewRenderer::PrepareForItem() in all ports

wxOSX and wxGTK previously used their own methods for handling the enabled
state and the attributes of the items being rendered, change them to reuse the
same methods as the generic implementation, i.e. SetEnabled() and SetAttr()
and remove the port-specific GtkSetAttr(), OSXApplyAttr() and so on.

This has the advantage of ensuring that the logic is the same for all
platforms (e.g. item enabled status wasn't handled in the same way in wxGTK as
in the other ports previously) and hopefully makes the code simpler by cutting
down on the number of virtual methods.

Notice that GtkSupportsAttrs() optimization was removed as it didn't seem to
be worth the bother (we basically saved a call to a virtual model method at a
price of a virtual renderer method call) and preserving it would have
complicated things needlessly.

include/wx/dvrenderers.h

@@ -115,10 +115,6 @@ 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; }
 
     // Prepare for rendering the value of the corresponding item in the given
@@ -128,22 +124,12 @@ public:
     // it is only passed to this method because the existing code already has
     // it and should probably be removed in the future.
     //
-    // Returns false if the value is missing (or invalid, i.e. has a wrong type
+    // Return true if this cell is non-empty or false otherwise (and also if
-    // differing from GetVariantType() of this renderer, in which case a debug
+    // the model returned a value of the wrong, i.e. different from our
-    // error is also logged as it indicates an error in the user code).
+    // GetVariantType(), type, in which case a debug error is also logged).
-    bool PrepareValue(const wxDataViewModel* model,
+    bool PrepareForItem(const wxDataViewModel *model,
-                      const wxDataViewItem& item,
+                        const wxDataViewItem& item,
-                      unsigned column);
+                        unsigned column);
-
-    // Prepare for rendering the given item by both calling PrepareValue() and
-    // setting up the attributes.
-    //
-    // This is currently only used in the generic version but should really be
-    // used everywhere, i.e. SetEnabled() and SetAttr() should be overridden in
-    // the native versions instead of adding platform-specific equivalents for
-    // them as it's done currently.
-    void PrepareForItem(const wxDataViewModel *model,
-                        const wxDataViewItem& item, unsigned column);
 
     // renderer properties:
     virtual void SetMode( wxDataViewCellMode mode ) = 0;
@@ -195,11 +181,17 @@ 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 PrepareValue() also used in StartEditing(): returns the value
+    // Helper of PrepareForItem() also used in StartEditing(): returns the
-    // checking that its type matches our GetVariantType().
+    // value checking that its type matches our GetVariantType().
     wxVariant CheckedGetValue(const wxDataViewModel* model,
                               const wxDataViewItem& item,
                               unsigned column) const;
@@ -318,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

@@ -808,34 +808,42 @@ wxDataViewRendererBase::CheckedGetValue(const wxDataViewModel* model,
 }
 
 bool
-wxDataViewRendererBase::PrepareValue(const wxDataViewModel* model,
+wxDataViewRendererBase::PrepareForItem(const wxDataViewModel *model,
-                                     const wxDataViewItem& item,
+                                       const wxDataViewItem& item,
-                                     unsigned column)
+                                       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);
 
-    if ( value.IsNull() )
+        // Also set up the attributes for this item if it's not empty.
-        return false;
+        wxDataViewItemAttr attr;
+        model->GetAttr(item, column, attr);
+        SetAttr(attr);
+    }
 
-    SetValue(value);
+    // 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;
 }
 
-void wxDataViewRendererBase::PrepareForItem(const wxDataViewModel *model,
-                                            const wxDataViewItem& item,
-                                            unsigned column)
-{
-    if ( !PrepareValue(model, item, column) )
-        return;
-
-    wxDataViewItemAttr attr;
-    model->GetAttr(item, column, attr);
-    SetAttr(attr);
-
-    SetEnabled(model->IsEnabled(item, column));
-}
-
 
 int wxDataViewRendererBase::GetEffectiveAlignment() const
 {
@@ -991,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
@@ -2923,37 +2945,7 @@ static void wxGtkTreeCellDataFunc( GtkTreeViewColumn *WXUNUSED(column),
             return;
     }
 
-    cell->PrepareValue(wx_model, item, column);
+    cell->PrepareForItem(wx_model, item, column);
-
-    // deal with disabled items
-    bool enabled = wx_model->IsEnabled( item, column );
-
-    // 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, column, 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

@@ -1419,7 +1419,7 @@ 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."));
-      if (dataViewColumnPtr->GetRenderer()->PrepareValue(dataViewCtrlPtr->GetModel(),wxDataViewItem(reinterpret_cast<void*>(itemID)),dataViewColumnPtr->GetModelColumn()))
+      if (dataViewColumnPtr->GetRenderer()->PrepareForItem(dataViewCtrlPtr->GetModel(),wxDataViewItem(reinterpret_cast<void*>(itemID)),dataViewColumnPtr->GetModelColumn()))
       {
         dataViewColumnPtr->GetRenderer()->GetNativeData()->SetItemDataRef(itemData);
         wxCHECK_MSG(dataViewColumnPtr->GetRenderer()->MacRender(),errDataBrowserNotConfigured,_("Rendering failed."));

src/osx/cocoa/dataview.mm

@@ -1796,40 +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
-    if ( !renderer->PrepareValue(model, dvItem, colIdx) )
+    if ( renderer->PrepareForItem(model, dvItem, colIdx) )
     {
-        // for consistency with the generic implementation, just handle missing
+        // and do render it in this case
-        // values as blank
+        renderer->MacRender();
-        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();
 }
 
 //
@@ -2708,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();
@@ -2777,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];
 }
@@ -2805,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);
 
 // ---------------------------------------------------------