Fix: was using locale to format SVG numbers.

Use normal attributes instead of style attribute, for greater viewer compatibility.
Tidy up the produced SVG: trim trailing zeros from numbers.
Stronger detection of invalid .ggr files.

diff --git a/src/extension/internal/gimpgrad.cpp b/src/extension/internal/gimpgrad.cpp
index ac3b807d14411dcf5120c3e7aa735a0bffbacf18..5e3924cef3f1aee0da0088d20d775a2792cf3db0 100644 (file)
--- a/src/extension/internal/gimpgrad.cpp
+++ b/src/extension/internal/gimpgrad.cpp
@@ -18,8 +18,12 @@
 #include <color-rgba.h>
 #include "io/sys.h"
 #include "extension/system.h"
+#include "svg/css-ostringstream.h"
+#include "svg/svg.h" // svg-color.h
 
 #include "gimpgrad.h"
+#include "streq.h"
+#include "strneq.h"
 
 namespace Inkscape {
 namespace Extension {
@@ -49,6 +53,14 @@ GimpGrad::unload (Inkscape::Extension::Extension *module)
     return;
 }
 
+static void
+append_css_num(Glib::ustring &str, double const num)
+{
+    CSSOStringStream stream;
+    stream << num;
+    str += stream.str();
+}
+
 /**
     \brief  A function to turn a color into a gradient stop
     \param  in_color  The color for the stop
@@ -61,29 +73,25 @@ GimpGrad::unload (Inkscape::Extension::Extension *module)
     hex values from 0 to 255 for color.  Otherwise mostly this is just
     turning the values into strings and returning it.
 */
-Glib::ustring
-GimpGrad::new_stop (ColorRGBA in_color, float location)
+static Glib::ustring
+stop_svg(ColorRGBA const in_color, double const location)
 {
-    char temp_string[25];
-    Glib::ustring mystring("<stop style=\"stop-color:#");
-
-    for (int i = 0; i < 3; i++) {
-        unsigned char temp;
+    Glib::ustring ret("<stop stop-color=\"");
 
-        temp = (unsigned char)(in_color[i] * 255.0);
+    char stop_color_css[16];
+    sp_svg_write_color(stop_color_css, sizeof(stop_color_css), in_color.getIntValue());
+    ret += stop_color_css;
+    ret += '"';
 
-        sprintf(temp_string, "%2.2X", temp);
-        mystring += temp_string;
+    if (in_color[3] != 1) {
+        ret += " stop-opacity=\"";
+        append_css_num(ret, in_color[3]);
+        ret += '"';
     }
-
-    mystring += ";stop-opacity:";
-    sprintf(temp_string, "%1.8f", in_color[3]);
-    mystring += temp_string;
-    mystring += ";\" offset=\"";
-    sprintf(temp_string, "%1.8f", location);
-    mystring += temp_string;
-    mystring += "\"/>\n";
-    return mystring;
+    ret += " offset=\"";
+    append_css_num(ret, location);
+    ret += "\"/>\n";
+    return ret;
 }
 
 /**
@@ -124,81 +132,136 @@ GimpGrad::new_stop (ColorRGBA in_color, float location)
 SPDocument *
 GimpGrad::open (Inkscape::Extension::Input *module, gchar const *filename)
 {
-    FILE * gradient;
-    // std::cout << "Open filename: " << filename << std::endl;
-
     Inkscape::IO::dump_fopen_call(filename, "I");
-    gradient = Inkscape::IO::fopen_utf8name(filename, "r");
-    if (gradient == NULL) return NULL;
-
-    char tempstr[1024];
-    if (fgets(tempstr, 1024, gradient) == 0) {
-        // std::cout << "Seems that the read failed" << std::endl;
-        fclose(gradient);
-        return NULL;
-    }
-
-    if (!strcmp(tempstr, "GIMP Gradient")) {
-        // std::cout << "This doesn't appear to be a GIMP gradient" << std::endl;
-        fclose(gradient);
-        return NULL;
-    }
-
-    if (fgets(tempstr, 1024, gradient) == 0) {
-        // std::cout << "Seems that the second read failed" << std::endl;
-        fclose(gradient);
+    FILE *gradient = Inkscape::IO::fopen_utf8name(filename, "r");
+    if (gradient == NULL) {
         return NULL;
     }
 
-    if (fgets(tempstr, 1024, gradient) == 0) {
-        // std::cout << "Seems that the third read failed" << std::endl;
-        fclose(gradient);
-        return NULL;
-    }
+    {
+        char tempstr[1024];
+        if (fgets(tempstr, 1024, gradient) == 0) {
+            // std::cout << "Seems that the read failed" << std::endl;
+            goto error;
+        }
+        if (!streq(tempstr, "GIMP Gradient\n")) {
+            // std::cout << "This doesn't appear to be a GIMP gradient" << std::endl;
+            goto error;
+        }
 
-    ColorRGBA last_color(-1.0, -1.0, -1.0, -1.0);
-    float lastlocation = -1.0;
-    Glib::ustring outsvg("<svg><defs><linearGradient>\n");
-    while (fgets(tempstr, 1024, gradient) != 0) {
-        float left, middle, right;
-        float temp_color[4];
-        int type;
-        int color;
-        gchar * end;
-
-        left = g_ascii_strtod(tempstr, &end);
-        middle = g_ascii_strtod(end, &end);
-        right = g_ascii_strtod(end, &end);
-
-        for (int i = 0; i < 4; i++) {
-            temp_color[i] = g_ascii_strtod(end, &end);
+        /* Name field. */
+        if (fgets(tempstr, 1024, gradient) == 0) {
+            // std::cout << "Seems that the second read failed" << std::endl;
+            goto error;
+        }
+        if (!strneq(tempstr, "Name: ", 6)) {
+            goto error;
+        }
+        /* Handle very long names.  (And also handle nul bytes gracefully: don't use strlen.) */
+        while (memchr(tempstr, '\n', sizeof(tempstr) - 1) == NULL) {
+            if (fgets(tempstr, sizeof(tempstr), gradient) == 0) {
+                goto error;
+            }
         }
-        ColorRGBA leftcolor(temp_color[0], temp_color[1], temp_color[2], temp_color[3]);
 
-        for (int i = 0; i < 4; i++) {
-            temp_color[i] = g_ascii_strtod(end, &end);
+        /* n. segments */
+        if (fgets(tempstr, 1024, gradient) == 0) {
+            // std::cout << "Seems that the third read failed" << std::endl;
+            goto error;
+        }
+        char *endptr = NULL;
+        long const n_segs = strtol(tempstr, &endptr, 10);
+        if ((*endptr != '\n')
+            || n_segs < 1) {
+            /* SVG gradients are allowed to have zero stops (treated as `none'), but gimp 2.2
+             * requires at least one segment (i.e. at least two stops) (see gimp_gradient_load in
+             * gimpgradient-load.c).  We try to use the same error handling as gimp, so that
+             * .ggr files that work in one program work in both programs. */
+            goto error;
         }
-        ColorRGBA rightcolor(temp_color[0], temp_color[1], temp_color[2], temp_color[3]);
 
-        sscanf(end, "%d %d", &type, &color);
+        ColorRGBA prev_color(-1.0, -1.0, -1.0, -1.0);
+        Glib::ustring outsvg("<svg><defs><linearGradient>\n");
+        long n_segs_found = 0;
+        double prev_right = 0.0;
+        while (fgets(tempstr, 1024, gradient) != 0) {
+            double dbls[3 + 4 + 4];
+            gchar *p = tempstr;
+            for (unsigned i = 0; i < G_N_ELEMENTS(dbls); ++i) {
+                gchar *end = NULL;
+                double const xi = g_ascii_strtod(p, &end);
+                if (!end || end == p || !g_ascii_isspace(*end)) {
+                    goto error;
+                }
+                if (xi < 0 || 1 < xi) {
+                    goto error;
+                }
+                dbls[i] = xi;
+                p = end + 1;
+            }
+
+            double const left = dbls[0];
+            if (left != prev_right) {
+                goto error;
+            }
+            double const middle = dbls[1];
+            if (!(left <= middle)) {
+                goto error;
+            }
+            double const right = dbls[2];
+            if (!(middle <= right)) {
+                goto error;
+            }
+
+            ColorRGBA const leftcolor(dbls[3], dbls[4], dbls[5], dbls[6]);
+            ColorRGBA const rightcolor(dbls[7], dbls[8], dbls[9], dbls[10]);
+            assert(11 == G_N_ELEMENTS(dbls));
+
+            /* Interpolation enums: curve shape and colour space. */
+            {
+                /* TODO: Currently we silently ignore type & color, assuming linear interpolation in
+                 * sRGB space (or whatever the default in SVG is).  See gimp/app/core/gimpgradient.c
+                 * for how gimp uses these.  We could use gimp functions to sample at a few points, and
+                 * add some intermediate stops to convert to the linear/sRGB interpolation */
+                int type; /* enum: linear, curved, sine, sphere increasing, sphere decreasing. */
+                int color_interpolation; /* enum: rgb, hsv anticlockwise, hsv clockwise. */
+                if (sscanf(p, "%d %d", &type, &color_interpolation) != 2) {
+                    continue;
+                }
+            }
+
+            if (prev_color != leftcolor) {
+                outsvg += stop_svg(leftcolor, left);
+            }
+            if (fabs(middle - .5 * (left + right)) > 1e-4) {
+                outsvg += stop_svg(leftcolor.average(rightcolor), middle);
+            }
+            outsvg += stop_svg(rightcolor, right);
+
+            prev_color = rightcolor;
+            prev_right = right;
+            ++n_segs_found;
+        }
+        if (prev_right != 1.0) {
+            goto error;
+        }
 
-        if (!(last_color == leftcolor) || left != lastlocation) {
-            outsvg += new_stop(leftcolor, left);
+        if (n_segs_found != n_segs) {
+            goto error;
         }
-        outsvg += new_stop(leftcolor.average(rightcolor), middle);
-        outsvg += new_stop(rightcolor, right);
 
-        last_color = rightcolor;
-        lastlocation = right;
-    }
+        outsvg += "</linearGradient></defs></svg>";
 
-    outsvg += "</linearGradient></defs></svg>";
+        // std::cout << "SVG Output: " << outsvg << std::endl;
 
-    // std::cout << "SVG Output: " << outsvg << std::endl;
+        fclose(gradient);
 
-    fclose(gradient);
+        return sp_document_new_from_mem(outsvg.c_str(), outsvg.length(), TRUE);
+    }
 
-    return sp_document_new_from_mem(outsvg.c_str(), outsvg.length(), TRUE);
+error:
+    fclose(gradient);
+    return NULL;
 }
 
 #include "clear-n_.h"

diff --git a/src/extension/internal/gimpgrad.h b/src/extension/internal/gimpgrad.h
index 0d0698e67c3a8d7a910125a586144d3f83986c9c..45b76dd6d42420da3f91494d8ec4055d087495b1 100644 (file)
--- a/src/extension/internal/gimpgrad.h
+++ b/src/extension/internal/gimpgrad.h
@@ -23,15 +23,12 @@ namespace Internal {
             just creates a namespace for the GIMP gradient plugin today.
 */
 class GimpGrad : public Inkscape::Extension::Implementation::Implementation {
-private:
-    Glib::ustring new_stop (ColorRGBA in_color, float location);
-
 public:
     bool load(Inkscape::Extension::Extension *module);
     void unload(Inkscape::Extension::Extension *module);
     SPDocument *open(Inkscape::Extension::Input *module, gchar const *filename);
 
-       static void init (void);
+    static void init();
 };
 
 
@@ -46,4 +43,4 @@ public:
   fill-column:99
   End:
 */
-// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4 :
+// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:encoding=utf-8:textwidth=99 :