[model] Rework Model behaviour with a filter

Currently ClutterModel::get_iter_at_row() ignores whether we have
a filter in place. This also extends to the get_n_rows() method.

The more consistent, more intuitive and surely more correct way to
handle a Model with a filter in place is to take into account the
presence of the filter itself -- that is:

  - get_n_rows() should take into account the filter and return the
    number of *filtered* rows

  - get_iter_at_row() should also take the filter into account and
    get the first non-filtered row

These two changes make the ClutterModel with a filter function
behave like a subset of the original Model without a filter in
place.

For instance, given a model with three rows:

  - [row 0]     <does not match filter>
  - [row 1]     <matches filter>
  - [row 2]     <matches filter>
  - [row 3]     <does not match filter>

The get_n_rows() method will return "2", since only two rows will
match the filter; the get_first_iter() method will ask for the
zero-eth row, which will return an iterator pointing to the contents
of row 1 (but the :row property of the iterator will be set to 0);
the get_last_iter() method will ask for the last row, which will
return an iterator pointing to the contents of row 2 (but the :row
property of the iterator will be set to 1).

This changes will hopefully make the Model API more consistent
in its usage whether there is a filter in place or not.

clutter/clutter-list-model.c

@@ -214,13 +214,11 @@ clutter_list_model_iter_is_first (ClutterModelIter *iter)
   ClutterModelIter *temp_iter;
   GSequence *sequence;
   GSequenceIter *begin, *end;
-  guint row;
 
   iter_default = CLUTTER_LIST_MODEL_ITER (iter);
   g_assert (iter_default->seq_iter != NULL);
 
   model = clutter_model_iter_get_model (iter);
-  row   = clutter_model_iter_get_row (iter);
 
   sequence = CLUTTER_LIST_MODEL (model)->priv->sequence;
 
@@ -234,7 +232,6 @@ clutter_list_model_iter_is_first (ClutterModelIter *iter)
   while (!g_sequence_iter_is_begin (begin))
     {
       CLUTTER_LIST_MODEL_ITER (temp_iter)->seq_iter = begin;
-      g_object_set (G_OBJECT (temp_iter), "row", row, NULL);
 
       if (clutter_model_filter_iter (model, temp_iter))
         {
@@ -243,7 +240,6 @@ clutter_list_model_iter_is_first (ClutterModelIter *iter)
         }
 
       begin = g_sequence_iter_next (begin);
-      row += 1;
     }
 
   g_object_unref (temp_iter);
@@ -264,7 +260,6 @@ clutter_list_model_iter_is_last (ClutterModelIter *iter)
   ClutterModel *model;
   GSequence *sequence;
   GSequenceIter *begin, *end;
-  guint row;
 
   iter_default = CLUTTER_LIST_MODEL_ITER (iter);
   g_assert (iter_default->seq_iter != NULL);
@@ -273,7 +268,6 @@ clutter_list_model_iter_is_last (ClutterModelIter *iter)
     return TRUE;
 
   model = clutter_model_iter_get_model (iter);
-  row   = clutter_model_iter_get_row (iter);
 
   sequence = CLUTTER_LIST_MODEL (model)->priv->sequence;
 
@@ -288,7 +282,6 @@ clutter_list_model_iter_is_last (ClutterModelIter *iter)
   while (!g_sequence_iter_is_begin (begin))
     {
       CLUTTER_LIST_MODEL_ITER (temp_iter)->seq_iter = begin;
-      g_object_set (G_OBJECT (temp_iter), "row", row, NULL);
 
       if (clutter_model_filter_iter (model, temp_iter))
         {
@@ -297,7 +290,6 @@ clutter_list_model_iter_is_last (ClutterModelIter *iter)
         }
 
       begin = g_sequence_iter_prev (begin);
-      row += 1;
     }
 
   g_object_unref (temp_iter);
@@ -323,7 +315,7 @@ clutter_list_model_iter_next (ClutterModelIter *iter)
   g_assert (iter_default->seq_iter != NULL);
 
   model = clutter_model_iter_get_model (iter);
-  row   = clutter_model_iter_get_row (iter) + 1;
+  row   = clutter_model_iter_get_row (iter);
 
   filter_next = g_sequence_iter_next (iter_default->seq_iter);
   g_assert (filter_next != NULL);
@@ -335,22 +327,20 @@ clutter_list_model_iter_next (ClutterModelIter *iter)
   while (!g_sequence_iter_is_end (filter_next))
     {
       CLUTTER_LIST_MODEL_ITER (temp_iter)->seq_iter = filter_next;
-      g_object_set (G_OBJECT (temp_iter), "row", row, NULL);
 
       if (clutter_model_filter_iter (model, temp_iter))
-        break;
+        {
+          row += 1;
+          break;
+        }
 
       filter_next = g_sequence_iter_next (filter_next);
-      row += 1;
     }
 
   g_object_unref (temp_iter);
 
-  /* We do this because the 'end_iter' is always *after* the last valid iter.
+  if (g_sequence_iter_is_end (filter_next))
-   * Otherwise loops will go on forever
+    row += 1;
-   */
-  if (filter_next == iter_default->seq_iter)
-    filter_next = g_sequence_iter_next (filter_next);
 
   /* update the iterator and return it */
   g_object_set (G_OBJECT (iter_default), "model", model, "row", row, NULL);
@@ -372,7 +362,7 @@ clutter_list_model_iter_prev (ClutterModelIter *iter)
   g_assert (iter_default->seq_iter != NULL);
 
   model = clutter_model_iter_get_model (iter);
-  row   = clutter_model_iter_get_row (iter) - 1;
+  row   = clutter_model_iter_get_row (iter);
 
   filter_prev = g_sequence_iter_prev (iter_default->seq_iter);
   g_assert (filter_prev != NULL);
@@ -384,22 +374,20 @@ clutter_list_model_iter_prev (ClutterModelIter *iter)
   while (!g_sequence_iter_is_begin (filter_prev))
     {
       CLUTTER_LIST_MODEL_ITER (temp_iter)->seq_iter = filter_prev;
-      g_object_set (G_OBJECT (temp_iter), "row", row, NULL);
 
       if (clutter_model_filter_iter (model, temp_iter))
-        break;
+        {
+          row -= 1;
+          break;
+        }
 
       filter_prev = g_sequence_iter_prev (filter_prev);
-      row -= 1;
     }
 
   g_object_unref (temp_iter);
 
-  /* We do this because the 'end_iter' is always *after* the last valid iter.
+  if (g_sequence_iter_is_begin (filter_prev))
-   * Otherwise loops will go on forever
+    row -= 1;
-   */
-  if (filter_prev == iter_default->seq_iter)
-    filter_prev = g_sequence_iter_prev (filter_prev);
 
   /* update the iterator and return it */
   g_object_set (G_OBJECT (iter_default), "model", model, "row", row, NULL);
@@ -466,16 +454,61 @@ clutter_list_model_get_iter_at_row (ClutterModel *model,
 {
   ClutterListModel *model_default = CLUTTER_LIST_MODEL (model);
   GSequence *sequence = model_default->priv->sequence;
+  gint seq_length = g_sequence_get_length (sequence);
   ClutterListModelIter *retval;
 
-  if (row >= g_sequence_get_length (sequence))
+  if (row >= seq_length)
     return NULL;
 
   retval = g_object_new (CLUTTER_TYPE_LIST_MODEL_ITER,
                          "model", model,
                          "row", row,
                          NULL);
-  retval->seq_iter = g_sequence_get_iter_at_pos (sequence, row);
+
+  /* short-circuit in case we don't have a filter in place */
+  if (!clutter_model_get_filter_set (model))
+    {
+      retval->seq_iter = g_sequence_get_iter_at_pos (sequence, row);
+
+      return CLUTTER_MODEL_ITER (retval);
+    }
+
+  if (row == 0)
+    {
+      GSequenceIter *filter_next;
+
+      filter_next = g_sequence_get_begin_iter (sequence);
+      g_assert (filter_next != NULL);
+
+      while (!g_sequence_iter_is_end (filter_next))
+        {
+          retval->seq_iter = filter_next;
+
+          if (clutter_model_filter_iter (model, CLUTTER_MODEL_ITER (retval)))
+            break;
+
+          filter_next = g_sequence_iter_next (filter_next);
+        }
+    }
+  else
+    {
+      GSequenceIter *filter_prev;
+
+      filter_prev = g_sequence_get_end_iter (sequence);
+      g_assert (filter_prev != NULL);
+
+      filter_prev = g_sequence_iter_prev (filter_prev);
+
+      while (!g_sequence_iter_is_begin (filter_prev))
+        {
+          retval->seq_iter = filter_prev;
+
+          if (clutter_model_filter_iter (model, CLUTTER_MODEL_ITER (retval)))
+            break;
+
+          filter_prev = g_sequence_iter_prev (filter_prev);
+        }
+    }
 
   return CLUTTER_MODEL_ITER (retval);
 }
@@ -507,7 +540,7 @@ clutter_list_model_insert_row (ClutterModel *model,
   if (index_ < 0)
     {
       seq_iter = g_sequence_append (sequence, array);
-      pos = g_sequence_get_length (sequence);
+      pos = g_sequence_get_length (sequence) - 1;
     }
   else if (index_ == 0)
     {
@@ -574,14 +607,6 @@ clutter_list_model_remove_row (ClutterModel *model,
     }
 }
 
-static guint
-clutter_list_model_get_n_rows (ClutterModel *model)
-{
-  ClutterListModel *model_default = CLUTTER_LIST_MODEL (model);
-
-  return g_sequence_get_length (model_default->priv->sequence);
-}
-
 typedef struct
 {
   ClutterModel *model;
@@ -668,7 +693,6 @@ clutter_list_model_class_init (ClutterListModelClass *klass)
 
   gobject_class->finalize = clutter_list_model_finalize;
 
-  model_class->get_n_rows      = clutter_list_model_get_n_rows;
   model_class->get_iter_at_row = clutter_list_model_get_iter_at_row;
   model_class->insert_row      = clutter_list_model_insert_row;
   model_class->remove_row      = clutter_list_model_remove_row;

clutter/clutter-model.c

@@ -208,28 +208,6 @@ clutter_model_real_get_n_columns (ClutterModel *model)
   return model->priv->n_columns;
 }
 
-static guint
-clutter_model_real_get_n_rows (ClutterModel *model)
-{
-  ClutterModelIter *iter;
-  guint n_rows = 0;
-
-  iter = clutter_model_get_first_iter (model);
-  if (!iter)
-    return 0;
-
-  while (!clutter_model_iter_is_last (iter))
-    {
-      n_rows += 1;
-
-      clutter_model_iter_next (iter);
-    }
-
-  g_object_unref (iter);
-
-  return n_rows;
-}
-
 static void 
 clutter_model_finalize (GObject *object)
 {
@@ -289,7 +267,6 @@ clutter_model_class_init (ClutterModelClass *klass)
   klass->get_column_name  = clutter_model_real_get_column_name;
   klass->get_column_type  = clutter_model_real_get_column_type;
   klass->get_n_columns    = clutter_model_real_get_n_columns;
-  klass->get_n_rows       = clutter_model_real_get_n_rows;
 
   /**
    * ClutterModel:filter-set:
@@ -519,7 +496,7 @@ clutter_model_filter_row (ClutterModel *model,
     return TRUE;
 
   iter = clutter_model_get_iter_at_row (model, row);
-  if (!iter)
+  if (iter == NULL)
     return FALSE;
 
   res = priv->filter_func (model, iter, priv->filter_data);
@@ -1162,6 +1139,10 @@ clutter_model_get_column_type (ClutterModel *model,
  *
  * Retrieves a #ClutterModelIter representing the row at the given index.
  *
+ * If a filter function has been set using clutter_model_set_filter()
+ * then the @model implementation will return the first non filtered
+ * row.
+ *
  * Return value: (transfer full): A new #ClutterModelIter, or %NULL if @row was
  *   out of bounds. When done using the iterator object, call g_object_unref()
  *   to deallocate its resources
@@ -1188,49 +1169,65 @@ clutter_model_get_iter_at_row (ClutterModel *model,
  * clutter_model_get_first_iter:
  * @model: a #ClutterModel
  *
- * Retrieves a #ClutterModelIter representing the first row in @model.
+ * Retrieves a #ClutterModelIter representing the first non-filtered
+ * row in @model.
  *
- * Return value: (transfer full): A new #ClutterModelIter. Call g_object_unref() when
+ * Return value: (transfer full): A new #ClutterModelIter.
- *   done using it
+ *   Call g_object_unref() when done using it
  *
  * Since: 0.6
  */
 ClutterModelIter *
 clutter_model_get_first_iter (ClutterModel *model)
 {
+  ClutterModelIter *retval;
+
   g_return_val_if_fail (CLUTTER_IS_MODEL (model), NULL);
 
-  return clutter_model_get_iter_at_row (model, 0);
+  retval = clutter_model_get_iter_at_row (model, 0);
+  if (retval != NULL)
+    {
+      g_assert (clutter_model_filter_iter (model, retval) != FALSE);
+      g_assert (clutter_model_iter_get_row (retval) == 0);
+    }
+
+  return retval;
 }
 
 /**
  * clutter_model_get_last_iter:
  * @model: a #ClutterModel
  *
- * Retrieves a #ClutterModelIter representing the last row in @model.
+ * Retrieves a #ClutterModelIter representing the last non-filtered
+ * row in @model.
  *
- * Return value: (transfer full): A new #ClutterModelIter. Call g_object_unref() when
+ * Return value: (transfer full): A new #ClutterModelIter.
- *   done using it
+ *   Call g_object_unref() when done using it
  *
  * Since: 0.6
  */
 ClutterModelIter *
 clutter_model_get_last_iter (ClutterModel *model)
 {
+  ClutterModelIter *retval;
   guint length;
 
   g_return_val_if_fail (CLUTTER_IS_MODEL (model), NULL);
 
   length = clutter_model_get_n_rows (model);
+  retval = clutter_model_get_iter_at_row (model, length - 1);
+  if (retval != NULL)
+    g_assert (clutter_model_filter_iter (model, retval) != FALSE);
 
-  return clutter_model_get_iter_at_row (model, length - 1);
+  return retval;
 }
 
 /**
  * clutter_model_get_n_rows:
  * @model: a #ClutterModel
  *
- * Retrieves the number of rows inside @model.
+ * Retrieves the number of rows inside @model, eventually taking
+ * into account any filtering function set using clutter_model_set_filter().
  *
  * Return value: The length of the @model. If there is a filter set, then
  *   the length of the filtered @model is returned.
@@ -1240,9 +1237,35 @@ clutter_model_get_last_iter (ClutterModel *model)
 guint
 clutter_model_get_n_rows (ClutterModel *model)
 {
+  ClutterModelClass *klass;
+  guint row_count;
+
   g_return_val_if_fail (CLUTTER_IS_MODEL (model), 0);
 
-  return CLUTTER_MODEL_GET_CLASS (model)->get_n_rows (model);
+  klass = CLUTTER_MODEL_GET_CLASS (model);
+  if (klass->get_n_rows)
+    row_count = klass->get_n_rows (model);
+  else
+    {
+      ClutterModelIter *iter;
+
+      iter = clutter_model_get_first_iter (model);
+      if (iter == NULL)
+        return 0;
+
+      row_count = 0;
+      while (!clutter_model_iter_is_last (iter))
+        {
+          if (clutter_model_filter_iter (model, iter))
+            row_count += 1;
+
+          iter = clutter_model_iter_next (iter);
+        }
+
+      g_object_unref (iter);
+    }
+
+  return row_count;
 }
 
 
@@ -1358,6 +1381,9 @@ clutter_model_set_sort (ClutterModel         *model,
   ClutterModelPrivate *priv;
     
   g_return_if_fail (CLUTTER_IS_MODEL (model));
+  g_return_if_fail ((func != NULL && column >= 0) ||
+                    (func == NULL && column == -1));
+
   priv = model->priv;
 
   if (priv->sort_notify)
@@ -1604,7 +1630,8 @@ clutter_model_iter_set_property (GObject      *object,
 static void
 clutter_model_iter_class_init (ClutterModelIterClass *klass)
 {
-  GObjectClass        *gobject_class = G_OBJECT_CLASS (klass);
+  GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
+  GParamSpec *pspec;
 
   gobject_class->get_property = clutter_model_iter_get_property;
   gobject_class->set_property = clutter_model_iter_set_property;
@@ -1628,28 +1655,26 @@ clutter_model_iter_class_init (ClutterModelIterClass *klass)
    *
    * Since: 0.6
    */
-  g_object_class_install_property (gobject_class,
+  pspec = g_param_spec_object ("model",
-                                   ITER_PROP_MODEL,
+                               "Model",
-                                   g_param_spec_object ("model",
+                               "The model to which the iterator belongs to",
-                                                        "Model",
+                               CLUTTER_TYPE_MODEL,
-                                                        "The model to which the iterator belongs to",
+                               CLUTTER_PARAM_READWRITE);
-                                                        CLUTTER_TYPE_MODEL,
+  g_object_class_install_property (gobject_class, ITER_PROP_MODEL, pspec);
-                                                        CLUTTER_PARAM_READWRITE));
 
-   /**
+  /**
    * ClutterModelIter:row:
    *
    * The row number to which this iter points to.
    *
    * Since: 0.6
    */
-  g_object_class_install_property (gobject_class,
+  pspec = g_param_spec_uint ("row",
-                                   ITER_PROP_ROW,
+                             "Row",
-                                   g_param_spec_uint ("row",
+                             "The row to which the iterator points to",
-                                                      "Row",
+                             0, G_MAXUINT, 0,
-                                                      "The row to which the iterator points to",
+                             CLUTTER_PARAM_READWRITE);
-                                                      0, G_MAXUINT, 0,
+  g_object_class_install_property (gobject_class, ITER_PROP_ROW, pspec);
-                                                      CLUTTER_PARAM_READWRITE));
   
   g_type_class_add_private (gobject_class, sizeof (ClutterModelIterPrivate));
 }

clutter/clutter-model.h

@@ -129,7 +129,7 @@ struct _ClutterModel
  * @get_iter_at_row: virtual function for returning an iterator for the
  *   given row
  * @get_n_rows: virtual function for returning the number of rows
- *   of the model, not considering any filter function if present
+ *   of the model
  * @get_n_columns: virtual function for retuning the number of columns
  *   of the model
  * @resort: virtual function for sorting the model using the passed