[gnumeric] GnmColor: avoid creating bogus object in sheet-style.



commit 14492709b74979065a3ab11b66292dcbc9cc8a8b
Author: Morten Welinder <terra gnome org>
Date:   Fri Mar 17 21:39:05 2017 -0400

    GnmColor: avoid creating bogus object in sheet-style.
    
    That was probably never a good idea, but it broke GnmColor and thus
    GnmBorder equality.

 ChangeLog                 |    9 ++++++++
 NEWS                      |    1 +
 src/sheet-style.c         |   25 +++++------------------
 src/style-color.c         |   47 ++++++++++++++++++++++++--------------------
 src/style-color.h         |    1 +
 test/t9002-ssdiff-self.pl |    4 ---
 6 files changed, 43 insertions(+), 44 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 5fef8c0..df102c3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2017-03-17  Morten Welinder  <terra gnome org>
 
+       * src/sheet-style.c (sheet_style_init_size): Don't hand-create a
+       bogus GnmColor here.  The fake object prevents GnmColor equality
+       from working which in turn prevents GnmBorder equality from
+       working.
+
+       * src/style-color.c (gnm_color_make): Rename from
+       gnm_color_new_uninterned and handle caching here.
+       (gnm_color_new_auto): New function.
+
        * src/ssdiff.c (main): Exit 2 on error, 1 of diffs, 0 if no diff.
        (compare_corresponding_cells): Fall back to string comparison.
        References like Sheet1!A1 should match even if they, obviously,
diff --git a/NEWS b/NEWS
index 60a8fd5..46b4d0d 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,7 @@ Morten:
          conditional formats, and validations.
        * Make exit code from ssdiff follow diff's.
        * Fix ssdiff with inter-sheet references.
+       * Fix style border colour sharing.
 
 --------------------------------------------------------------------------
 Gnumeric 1.12.33
diff --git a/src/sheet-style.c b/src/sheet-style.c
index d228789..d04727b 100644
--- a/src/sheet-style.c
+++ b/src/sheet-style.c
@@ -681,14 +681,7 @@ sheet_style_init_size (Sheet *sheet, int cols, int rows)
        sheet->style_data = g_new (GnmSheetStyleData, 1);
        sheet->style_data->style_hash = sh_create ();
 
-       {
-               GnmColor *ap = style_color_auto_pattern ();
-#warning "FIXME: Allocating a GnmColor here is dubious."
-               sheet->style_data->auto_pattern_color = g_new (GnmColor, 1);
-               *sheet->style_data->auto_pattern_color = *ap;
-               sheet->style_data->auto_pattern_color->ref_count = 1;
-               style_color_unref (ap);
-       }
+       sheet->style_data->auto_pattern_color = style_color_auto_pattern ();
 
        default_style =  gnm_style_new_default ();
 #if 0
@@ -833,7 +826,7 @@ sheet_style_shutdown (Sheet *sheet)
 /**
  * sheet_style_set_auto_pattern_color:
  * @sheet: The sheet
- * @grid_color: The color
+ * @grid_color: (transfer full): The color
  *
  * Set the color for rendering auto colored patterns in this sheet.
  * Absorbs a reference to @pattern_color;
@@ -841,17 +834,11 @@ sheet_style_shutdown (Sheet *sheet)
 void
 sheet_style_set_auto_pattern_color (Sheet *sheet, GnmColor *pattern_color)
 {
-       GnmColor *apc;
-       int ref_count;
-
        g_return_if_fail (IS_SHEET (sheet));
        g_return_if_fail (sheet->style_data != NULL);
 
-       apc = sheet->style_data->auto_pattern_color;
-       ref_count = apc->ref_count;
-       *apc = *pattern_color;
-       apc->is_auto = TRUE;
-       apc->ref_count = ref_count;
+       style_color_unref (sheet->style_data->auto_pattern_color);
+       sheet->style_data->auto_pattern_color = gnm_color_new_auto (pattern_color->go_color);
        style_color_unref (pattern_color);
 }
 
@@ -859,8 +846,8 @@ sheet_style_set_auto_pattern_color (Sheet *sheet, GnmColor *pattern_color)
  * sheet_style_get_auto_pattern_color:
  * @sheet: the sheet
  *
- * Caller receives a reference to the result.
- * Returns the color for rendering auto colored patterns in this sheet.
+ * Returns: (transfer full): the color for rendering auto colored patterns
+ * in this sheet.
  **/
 GnmColor *
 sheet_style_get_auto_pattern_color (Sheet const *sheet)
diff --git a/src/style-color.c b/src/style-color.c
index 6a65a1e..1f91088 100644
--- a/src/style-color.c
+++ b/src/style-color.c
@@ -21,13 +21,25 @@ static GnmColor *sc_auto_font;
 static GnmColor *sc_auto_pattern;
 
 static GnmColor *
-gnm_color_new_uninterned (GOColor c, gboolean is_auto)
+gnm_color_make (GOColor c, gboolean is_auto)
 {
-       GnmColor *sc = g_new (GnmColor, 1);
+       GnmColor key, *sc;
 
-       sc->go_color = c;
-       sc->is_auto = !!is_auto;
-       sc->ref_count = 1;
+       is_auto = !!is_auto;
+
+       key.go_color = c;
+       key.is_auto = is_auto;
+
+       sc = g_hash_table_lookup (style_color_hash, &key);
+       if (!sc) {
+               sc = g_new (GnmColor, 1);
+               sc->go_color = c;
+               sc->is_auto = is_auto;
+               sc->ref_count = 1;
+
+               g_hash_table_insert (style_color_hash, sc, sc);
+       } else
+               sc->ref_count++;
 
        return sc;
 }
@@ -77,20 +89,13 @@ gnm_color_new_rgb8 (guint8 red, guint8 green, guint8 blue)
 GnmColor *
 gnm_color_new_go (GOColor c)
 {
-       GnmColor *sc;
-       GnmColor key;
-
-       key.go_color = c;
-       key.is_auto = FALSE;
-
-       sc = g_hash_table_lookup (style_color_hash, &key);
-       if (!sc) {
-               sc = gnm_color_new_uninterned (c, FALSE);
-               g_hash_table_insert (style_color_hash, sc, sc);
-       } else
-               sc->ref_count++;
+       return gnm_color_make (c, FALSE);
+}
 
-       return sc;
+GnmColor *
+gnm_color_new_auto (GOColor c)
+{
+       return gnm_color_make (c, TRUE);
 }
 
 GnmColor *
@@ -130,7 +135,7 @@ GnmColor *
 style_color_auto_font (void)
 {
        if (!sc_auto_font)
-               sc_auto_font = gnm_color_new_uninterned (GO_COLOR_BLACK, TRUE);
+               sc_auto_font = gnm_color_new_auto (GO_COLOR_BLACK);
        return style_color_ref (sc_auto_font);
 }
 
@@ -143,7 +148,7 @@ GnmColor *
 style_color_auto_back (void)
 {
        if (!sc_auto_back)
-               sc_auto_back = gnm_color_new_uninterned (GO_COLOR_WHITE, TRUE);
+               sc_auto_back = gnm_color_new_auto (GO_COLOR_WHITE);
        return style_color_ref (sc_auto_back);
 }
 
@@ -156,7 +161,7 @@ GnmColor *
 style_color_auto_pattern (void)
 {
        if (!sc_auto_pattern)
-               sc_auto_pattern = gnm_color_new_uninterned (GO_COLOR_BLACK, TRUE);
+               sc_auto_pattern = gnm_color_new_auto (GO_COLOR_BLACK);
        return style_color_ref (sc_auto_pattern);
 }
 
diff --git a/src/style-color.h b/src/style-color.h
index 4d6fbbd..eca4bf1 100644
--- a/src/style-color.h
+++ b/src/style-color.h
@@ -25,6 +25,7 @@ GnmColor *gnm_color_new_rgb8  (guint8 red, guint8 green, guint8 blue);
 GnmColor *gnm_color_new_rgba8 (guint8 red, guint8 green, guint8 blue, guint8 alpha);
 GnmColor *gnm_color_new_pango (PangoColor const *c);
 GnmColor *gnm_color_new_gdk   (GdkRGBA const *c);
+GnmColor *gnm_color_new_auto  (GOColor c);
 GnmColor *style_color_auto_font (void);
 GnmColor *style_color_auto_back (void);
 GnmColor *style_color_auto_pattern (void);
diff --git a/test/t9002-ssdiff-self.pl b/test/t9002-ssdiff-self.pl
index cc7856f..7604cca 100755
--- a/test/t9002-ssdiff-self.pl
+++ b/test/t9002-ssdiff-self.pl
@@ -8,10 +8,6 @@ use GnumericTest;
 &message ("Check ssdiff on identical files");
 
 my @sources = &GnumericTest::corpus();
-# Remove stuff that currently fails.  (Not yet investigated.)
-@sources = grep { !/dbfuns\.xls/} @sources;
-@sources = grep { !/operator\.xls/} @sources;
-@sources = grep { !/cellstyle\.xlsx/} @sources;
 
 my $ngood = 0;
 my $nbad = 0;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]