Preserve paint styles with multiple components

diff --git a/src/extract-uri.cpp b/src/extract-uri.cpp
index b3923ed1e4dd64df75961b57e8c6b6affff7b627..8580270108bfafcf9322ff5964ede83cd93206d2 100644 (file)
--- a/src/extract-uri.cpp
+++ b/src/extract-uri.cpp
@@ -5,7 +5,7 @@
 
 // Functions as per 4.3.4 of CSS 2.1
 // http://www.w3.org/TR/CSS21/syndata.html#uri
-gchar *extract_uri(gchar const *s)
+gchar *extract_uri( gchar const *s, gchar const** endptr )
 {
     if (!s)
         return NULL;
@@ -18,6 +18,10 @@ gchar *extract_uri(gchar const *s)
 
     sb += 3;
 
+    if ( endptr ) {
+        *endptr = 0;
+    }
+
     // This first whitespace technically is not allowed.
     // Just left in for now for legacy behavior.
     while ( ( *sb == ' ' ) ||
@@ -47,6 +51,10 @@ gchar *extract_uri(gchar const *s)
         // we found the delimiter
         if ( *se ) {
             if ( delim == ')' ) {
+                if ( endptr ) {
+                    *endptr = se + 1;
+                }
+
                 // back up for any trailing whitespace
                 se--;
                 while ( ( se[-1] == ' ' ) ||
@@ -54,6 +62,7 @@ gchar *extract_uri(gchar const *s)
                 {
                     se--;
                 }
+
                 result = g_strndup(sb, se - sb + 1);
             } else {
                 gchar const* tail = se + 1;
@@ -63,6 +72,9 @@ gchar *extract_uri(gchar const *s)
                     tail++;
                 }
                 if ( *tail == ')' ) {
+                    if ( endptr ) {
+                        *endptr = tail + 1;
+                    }
                     result = g_strndup(sb, se - sb);
                 }
             }

diff --git a/src/extract-uri.h b/src/extract-uri.h
index 10def390458e5c8e335152d700e1d4eaa1dd6cc7..1975d9b3af3f97fe2716197fd15c1b349e8d80d9 100644 (file)
--- a/src/extract-uri.h
+++ b/src/extract-uri.h
@@ -3,7 +3,7 @@
 
 #include <glib/gtypes.h>
 
-gchar *extract_uri(gchar const *s);
+gchar *extract_uri(gchar const *s, gchar const** endptr = 0);
 
 
 #endif /* !SEEN_EXTRACT_URI_H */

diff --git a/src/sp-shape.cpp b/src/sp-shape.cpp
index 5c8e0922daa8d8c0164b7df629368bfa2242d3cf..9a74b7217373452bdf9be1dbbd6f4d78c4c2be16 100644 (file)
--- a/src/sp-shape.cpp
+++ b/src/sp-shape.cpp
@@ -1001,7 +1001,7 @@ sp_shape_set_marker (SPObject *object, unsigned int key, const gchar *value)
         return;
     }
 
-    SPObject *mrk = sp_uri_reference_resolve (SP_OBJECT_DOCUMENT (object), value);
+    SPObject *mrk = sp_css_uri_reference_resolve (SP_OBJECT_DOCUMENT (object), value);
     if (mrk != shape->marker[key]) {
         if (shape->marker[key]) {
             SPItemView *v;

diff --git a/src/style-test.h b/src/style-test.h
index 2f923dfa0c400ce7623ff04f82119b1f35c0f600..a2d5fcf24459bdcf7aa4d2b56495811cf654300d 100644 (file)
--- a/src/style-test.h
+++ b/src/style-test.h
@@ -4,11 +4,50 @@
 
 #include <cxxtest/TestSuite.h>
 
+#include "test-helpers.h"
+
 #include "style.h"
 
 class StyleTest : public CxxTest::TestSuite
 {
 public:
+    SPDocument* _doc;
+
+    StyleTest() :
+        _doc(0)
+    {
+    }
+
+    virtual ~StyleTest()
+    {
+        if ( _doc )
+        {
+            sp_document_unref( _doc );
+            _doc = 0;
+        }
+    }
+
+    static void createSuiteSubclass( StyleTest*& dst )
+    {
+        dst = new StyleTest();
+    }
+
+// createSuite and destroySuite get us per-suite setup and teardown
+// without us having to worry about static initialization order, etc.
+    static StyleTest *createSuite()
+    {
+        StyleTest* suite = Inkscape::createSuiteAndDocument<StyleTest>( createSuiteSubclass );
+        return suite;
+    }
+
+    static void destroySuite( StyleTest *suite )
+    {
+        delete suite;
+    }
+
+    // ---------------------------------------------------------------
+    // ---------------------------------------------------------------
+    // ---------------------------------------------------------------
 
     void testOne()
     {
@@ -26,27 +65,25 @@ public:
 
             TestCase("fill:rgb(100%, 0%, 100%)", "fill:#ff00ff"),
             // TODO - fix this to preserve the string
-//             TestCase("fill:url(#painter) rgb(100%, 0%, 100%)",
-//                      "fill:url(#painter) #ff00ff", "#painter"),
+            TestCase("fill:url(#painter) rgb(100%, 0%, 100%)",
+                     "fill:url(#painter) #ff00ff", "#painter"),
 
             TestCase("fill:rgb(255, 0, 255)", "fill:#ff00ff"),
             // TODO - fix this to preserve the string
-//             TestCase("fill:url(#painter) rgb(255, 0, 255)",
-//                      "fill:url(#painter) #ff00ff", "#painter"),
+            TestCase("fill:url(#painter) rgb(255, 0, 255)",
+                     "fill:url(#painter) #ff00ff", "#painter"),
 
 
 //             TestCase("fill:#ff00ff icc-color(colorChange, 0.1, 0.5, 0.1)"),
 
             TestCase("fill:url(#painter)", 0, "#painter"),
-//             TestCase("fill:url(#painter) none", 0, "#painter"),
-//             TestCase("fill:url(#painter) currentColor", 0, "#painter"),
-//             TestCase("fill:url(#painter) #ff00ff", 0, "#painter"),
+            TestCase("fill:url(#painter) none", 0, "#painter"),
+            TestCase("fill:url(#painter) currentColor", 0, "#painter"),
+            TestCase("fill:url(#painter) #ff00ff", 0, "#painter"),
 //             TestCase("fill:url(#painter) rgb(100%, 0%, 100%)", 0, "#painter"),
 //             TestCase("fill:url(#painter) rgb(255, 0, 255)", 0, "#painter"),
 
-//             TestCase("fill:url(#painter) #ff00ff icc-color(colorChange, 0.1, 0.5, 0.1)",
-//             "fill:url(#painter) #ff00ff icc-color(colorChange, 0.10000000000000001, 0.50000000000000000, 0.10000000000000001)", "#painter"),
-//             TestCase("fill:url(#painter) #ff00ff icc-color(colorChange, 0.1, 0.5, 0.1)", 0, "#painter"),
+            TestCase("fill:url(#painter) #ff00ff icc-color(colorChange, 0.1, 0.5, 0.1)", 0, "#painter"),
 
 //             TestCase("fill:url(#painter) inherit", 0, "#painter"),
             TestCase("fill:inherit"),
@@ -54,18 +91,18 @@ public:
         };
 
         for ( gint i = 0; cases[i].src; i++ ) {
-            SPStyle *style = sp_style_new(NULL);
+            SPStyle *style = sp_style_new(_doc);
             TS_ASSERT(style);
             if ( style ) {
                 sp_style_merge_from_style_string( style, cases[i].src );
 
                 if ( cases[i].uri ) {
-                    TS_ASSERT( style->fill.value.href );
+                    TSM_ASSERT( cases[i].src, style->fill.value.href );
                     if ( style->fill.value.href ) {
                         TS_ASSERT_EQUALS( style->fill.value.href->getURI()->toString(), std::string(cases[i].uri) );
                     }
                 } else {
-                    TS_ASSERT( !style->fill.value.href );
+                    TS_ASSERT( !style->fill.value.href || !style->fill.value.href->getObject() );
                 }
 
                 gchar *str0_set = sp_style_write_string( style, SP_STYLE_FLAG_IFSET );

diff --git a/src/style.cpp b/src/style.cpp
index df3a470a53d389307ca545271a3d0c5fee56b7af..5817df3428e746eb08736edae748ff5ba45c599d 100644 (file)
--- a/src/style.cpp
+++ b/src/style.cpp
@@ -2982,64 +2982,73 @@ sp_style_read_icolor(SPIPaint *paint, gchar const *str, SPStyle *style, SPDocume
 static void
 sp_style_read_ipaint(SPIPaint *paint, gchar const *str, SPStyle *style, SPDocument *document)
 {
-    while (isspace(*str)) {
+    while (g_ascii_isspace(*str)) {
         ++str;
     }
 
+    if (paint->value.href && paint->value.href->getObject()) {
+        paint->value.href->detach();
+    }
+    paint->colorSet = FALSE;
+    paint->currentcolor = FALSE;
+    paint->noneSet = FALSE;
+
     if (streq(str, "inherit")) {
         paint->set = TRUE;
         paint->inherit = TRUE;
-        paint->currentcolor = FALSE;
-    } else if (streq(str, "currentColor") && paint != &style->color) {
-        paint->set = TRUE;
-        paint->inherit = FALSE;
-        paint->currentcolor = TRUE;
-    } else if (streq(str, "none") && paint != &style->color) {
+    } else {
         paint->type = SP_PAINT_TYPE_NONE;
-        paint->set = TRUE;
         paint->inherit = FALSE;
-        paint->currentcolor = FALSE;
-    } else if (strneq(str, "url", 3) && paint != &style->color) {
-        gchar *uri = extract_uri(str);
-        if(uri == NULL || uri[0] == '\0') {
-            paint->type = SP_PAINT_TYPE_NONE;
-            return;
-        }
-        paint->type = SP_PAINT_TYPE_PAINTSERVER;
-        paint->set = TRUE;
-        paint->inherit = FALSE;
-        paint->currentcolor = FALSE;
+        if ( strneq(str, "url", 3) ) {
+            gchar *uri = extract_uri( str, &str );
+            while ( g_ascii_isspace(*str) ) {
+                ++str;
+            }
+            // TODO check on and comment the comparrison "paint != &style->color".
+            if ( uri && *uri && (paint != &style->color) ) {
+                paint->type = SP_PAINT_TYPE_PAINTSERVER;
+                paint->set = TRUE;
+
+                // it may be that this style's SPIPaint has not yet created its URIReference;
+                // now that we have a document, we can create it here
+                if (!paint->value.href && document) {
+                    paint->value.href = new SPPaintServerReference(document);
+                    paint->value.href->changedSignal().connect(sigc::bind(sigc::ptr_fun((paint == &style->fill)? sp_style_fill_paint_server_ref_changed : sp_style_stroke_paint_server_ref_changed), style));
+                }
 
-        // it may be that this style's SPIPaint has not yet created its URIReference;
-        // now that we have a document, we can create it here
-        if (!paint->value.href && document) {
-            paint->value.href = new SPPaintServerReference(document);
-            paint->value.href->changedSignal().connect(sigc::bind(sigc::ptr_fun((paint == &style->fill)? sp_style_fill_paint_server_ref_changed : sp_style_stroke_paint_server_ref_changed), style));
+                // TODO check what this does in light of move away from union
+                sp_style_set_ipaint_to_uri_string (style, paint, uri);
+            }
+            g_free( uri );
         }
 
-        sp_style_set_ipaint_to_uri_string (style, paint, uri);
-
-        g_free (uri);
-        
-    } else {
-        guint32 const rgb0 = sp_svg_read_color(str, &str, 0xff);
-        if (rgb0 != 0xff) {
-            paint->type = SP_PAINT_TYPE_COLOR;
-            sp_color_set_rgb_rgba32(&paint->value.color, rgb0);
+        if (streq(str, "currentColor") && paint != &style->color) {
             paint->set = TRUE;
-            paint->inherit = FALSE;
-            paint->currentcolor = FALSE;
+            paint->currentcolor = TRUE;
+        } else if (streq(str, "none") && paint != &style->color) {
+            paint->set = TRUE;
+            paint->noneSet = TRUE;
+        } else {
+            guint32 const rgb0 = sp_svg_read_color(str, &str, 0xff);
+            if (rgb0 != 0xff) {
+                if ( paint->type != SP_PAINT_TYPE_PAINTSERVER ) {
+                    paint->type = SP_PAINT_TYPE_COLOR;
+                }
+                sp_color_set_rgb_rgba32(&paint->value.color, rgb0);
+                paint->set = TRUE;
+                paint->colorSet = true;
 
-            while (g_ascii_isspace(*str)) {
-                ++str;
-            }
-            if (strneq(str, "icc-color(", 10)) {
-                SVGICCColor* tmp = new SVGICCColor();
-                if ( ! sp_svg_read_icc_color( str, &str, tmp ) ) {
-                    delete tmp;
-                    tmp = 0;
+                while (g_ascii_isspace(*str)) {
+                    ++str;
+                }
+                if (strneq(str, "icc-color(", 10)) {
+                    SVGICCColor* tmp = new SVGICCColor();
+                    if ( ! sp_svg_read_icc_color( str, &str, tmp ) ) {
+                        delete tmp;
+                        tmp = 0;
+                    }
+                    paint->value.iccColor = tmp;
                 }
-                paint->value.iccColor = tmp;
             }
         }
     }
@@ -3538,43 +3547,66 @@ static gint
 sp_style_write_ipaint(gchar *b, gint const len, gchar const *const key,
                       SPIPaint const *const paint, SPIPaint const *const base, guint const flags)
 {
+    int retval = 0;
+
     if ((flags & SP_STYLE_FLAG_ALWAYS)
         || ((flags & SP_STYLE_FLAG_IFSET) && paint->set)
         || ((flags & SP_STYLE_FLAG_IFDIFF) && paint->set
             && (!base->set || sp_paint_differ(paint, base))))
     {
+        CSSOStringStream css;
+
         if (paint->inherit) {
-            return g_snprintf(b, len, "%s:inherit;", key);
-        } else if (paint->currentcolor) {
-            return g_snprintf(b, len, "%s:currentColor;", key);
+            css << "inherit";
         } else {
-            switch (paint->type) {
-                case SP_PAINT_TYPE_COLOR: {
-                    char color_buf[8];
-                    sp_svg_write_color(color_buf, sizeof(color_buf), sp_color_get_rgba32_ualpha(&paint->value.color, 0));
-                    if (paint->value.iccColor) {
-                        CSSOStringStream css;
-                        css << color_buf << " icc-color(" << paint->value.iccColor->colorProfile;
-                        for (vector<double>::const_iterator i(paint->value.iccColor->colors.begin()),
-                                 iEnd(paint->value.iccColor->colors.end());
-                             i != iEnd; ++i) {
-                            css << ", " << *i;
-                        }
-                        css << ')';
-                        return g_snprintf(b, len, "%s:%s;", key, css.gcharp());
-                    } else {
-                        return g_snprintf(b, len, "%s:%s;", key, color_buf);
-                    }
+            if ( paint->value.href && paint->value.href->getURI() ) {
+                const gchar* uri = paint->value.href->getURI()->toString();
+                css << "url(" << uri << ")";
+            }
+
+            if ( paint->noneSet ) {
+                if ( !css.str().empty() ) {
+                    css << " ";
                 }
-                case SP_PAINT_TYPE_PAINTSERVER:
-                    return g_snprintf(b, len, "%s:url(%s);", key, paint->value.href? paint->value.href->getURI()->toString() : "");
-                default:
-                    break;
+                css << "none";
+            }
+
+            if ( paint->currentcolor ) {
+                if ( !css.str().empty() ) {
+                    css << " ";
+                }
+                css << "currentColor";
             }
-            return g_snprintf(b, len, "%s:none;", key);
+
+            if ( paint->colorSet ) {
+                if ( !css.str().empty() ) {
+                    css << " ";
+                }
+                char color_buf[8];
+                sp_svg_write_color(color_buf, sizeof(color_buf), sp_color_get_rgba32_ualpha(&paint->value.color, 0));
+                css << color_buf;
+            }
+
+            if (paint->value.iccColor) {
+                if ( !css.str().empty() ) {
+                    css << " ";
+                }
+                css << "icc-color(" << paint->value.iccColor->colorProfile;
+                for (vector<double>::const_iterator i(paint->value.iccColor->colors.begin()),
+                         iEnd(paint->value.iccColor->colors.end());
+                     i != iEnd; ++i) {
+                    css << ", " << *i;
+                }
+                css << ')';
+            }
+        }
+
+        if ( !css.str().empty() ) {
+            retval = g_snprintf( b, len, "%s:%s;", key, css.gcharp() );
         }
     }
-    return 0;
+
+    return retval;
 }
 
 

diff --git a/src/style.h b/src/style.h
index 2e928224b932518b740c02017ab6d11f570d4bbd..3a547bfb90b2e6431782690a8b86a1dd5feab940 100644 (file)
--- a/src/style.h
+++ b/src/style.h
@@ -159,6 +159,8 @@ struct SPIPaint {
     unsigned inherit : 1;
     unsigned currentcolor : 1;
     unsigned type : 2;
+    unsigned int colorSet : 1;
+    unsigned int noneSet : 1;
     struct {
         SPPaintServerReference *href; 
         SPColor color;

diff --git a/src/uri-references.cpp b/src/uri-references.cpp
index c9482839d56a5452579583549712eb6ba1b9395d..7fc1f483801e9713edd791366fd9d75d4f37ab93 100644 (file)
--- a/src/uri-references.cpp
+++ b/src/uri-references.cpp
@@ -16,11 +16,10 @@
 #include "sp-object.h"
 #include "uri.h"
 #include "uri-references.h"
+#include "extract-uri.h"
 
 #include <sigc++/functors/mem_fun.h>
 
-static gchar *uri_to_id(SPDocument *document, const gchar *uri);
-
 namespace Inkscape {
 
 URIReference::URIReference(SPObject *owner)
@@ -133,48 +132,29 @@ void URIReference::_release(SPObject *obj) {
 
 } /* namespace Inkscape */
 
-static gchar *
-uri_to_id(SPDocument *document, const gchar *uri)
+SPObject* sp_css_uri_reference_resolve( SPDocument *document, const gchar *uri )
 {
-       const gchar *e;
-       gchar *id;
-       gint len;
-
-       g_return_val_if_fail (document != NULL, NULL);
-
-       if (!uri) return NULL;
-       /* fixme: xpointer, everything */
-       if (strncmp (uri, "url(#", 5)) return NULL;
-
-       e = uri + 5;
-       while (*e) {
-               if (*e == ')') break;
-               if (!isalnum (*e) && (*e != '_') && (*e != '-') && (*e != ':') && (*e != '.')) return NULL;
-               e += 1;
-               if (!*e) return NULL;
-       }
-
-       len = e - uri - 5;
-       if (len < 1) return NULL;
+    SPObject* ref = 0;
 
-       id = (gchar*)g_new(gchar, len + 1);
-       memcpy (id, uri + 5, len);
-       id[len] = '\0';
+    if ( document && uri && ( strncmp(uri, "url(", 4) == 0 )) {
+        gchar *trimmed = extract_uri( uri );
+        if ( trimmed ) {
+            ref = sp_uri_reference_resolve( document, trimmed );
+            g_free( trimmed );
+        }
+    }
 
-       return id;
+    return ref;
 }
 
 SPObject *
 sp_uri_reference_resolve (SPDocument *document, const gchar *uri)
 {
-       gchar *id;
+    SPObject* ref = 0;
 
-       id = uri_to_id(document, uri);
-       if (!id) return NULL;
+    if ( uri && (*uri == '#') ) {
+        ref = document->getObjectById( uri + 1 );
+    }
 
-       SPObject *ref;
-       ref = document->getObjectById(id);
-       g_free(id);
-       return ref;
+    return ref;
 }
-

diff --git a/src/uri-references.h b/src/uri-references.h
index 4a5b1516aa7b83a816033d7c7d67cf93d423a558..3955d5f009bd40739479b6b0aed6b089c49dfa4d 100644 (file)
--- a/src/uri-references.h
+++ b/src/uri-references.h
@@ -139,6 +139,11 @@ private:
 
 }
 
+/**
+ * Resolves an item referenced by a URI in CSS form contained in "url(...)"
+ */
+SPObject* sp_css_uri_reference_resolve( SPDocument *document, const gchar *uri );
+
 SPObject *sp_uri_reference_resolve (SPDocument *document, const gchar *uri);
 
 #endif