Nano SVG is still locale dependent

[zezic]

It looks like Nano SVG parser is still depends on locale. Here is the related issue: VCVRack/Rack#461

[wcout]

Reason is most likely that nanosvg.h uses sscanf() to read float values.

Here is a patch to replace them with locale independent code (using what's already available):

diff --git a/src/nanosvg.h b/src/nanosvg.h
index 33fe706..5ad039f 100644
--- a/src/nanosvg.h
+++ b/src/nanosvg.h
@@ -1422,8 +1422,7 @@ static unsigned int nsvg__parseColor(const char* str)
 
 static float nsvg__parseOpacity(const char* str)
 {
-	float val = 0;
-	sscanf(str, "%f", &val);
+	float val = nsvg__atof(str);
 	if (val < 0.0f) val = 0.0f;
 	if (val > 1.0f) val = 1.0f;
 	return val;
@@ -1431,8 +1430,7 @@ static float nsvg__parseOpacity(const char* str)
 
 static float nsvg__parseMiterLimit(const char* str)
 {
-	float val = 0;
-	sscanf(str, "%f", &val);
+	float val = nsvg__atof(str);
 	if (val < 0.0f) val = 0.0f;
 	return val;
 }
@@ -1463,9 +1461,9 @@ static int nsvg__parseUnits(const char* units)
 static NSVGcoordinate nsvg__parseCoordinateRaw(const char* str)
 {
 	NSVGcoordinate coord = {0, NSVG_UNITS_USER};
-	char units[32]="";
-	sscanf(str, "%f%31s", &coord.value, units);
-	coord.units = nsvg__parseUnits(units);
+	char buf[64];
+	coord.units = nsvg__parseUnits(nsvg__parseNumber(str, buf, 64));
+	coord.value = nsvg__atof(buf);
 	return coord;
 }
 
@@ -2505,7 +2503,25 @@ static void nsvg__parseSVG(NSVGparser* p, const char** attr)
 			} else if (strcmp(attr[i], "height") == 0) {
 				p->image->height = nsvg__parseCoordinate(p, attr[i + 1], 0.0f, 0.0f);
 			} else if (strcmp(attr[i], "viewBox") == 0) {
-				sscanf(attr[i + 1], "%f%*[%%, \t]%f%*[%%, \t]%f%*[%%, \t]%f", &p->viewMinx, &p->viewMiny, &p->viewWidth, &p->viewHeight);
+				const char *s = attr[i + 1];
+				char buf[64];
+				s = nsvg__parseNumber(s, buf, 64);
+				p->viewMinx = nsvg__atof(buf);
+				while (*s && (nsvg__isspace(*s) || *s == '%' || *s == ',')) s++;
+				if (*s) {
+					s = nsvg__parseNumber(s, buf, 64);
+					p->viewMiny = nsvg__atof(buf);
+					while (*s && (nsvg__isspace(*s) || *s == '%' || *s == ',')) s++;
+					if (*s) {
+						s = nsvg__parseNumber(s, buf, 64);
+						p->viewWidth = nsvg__atof(buf);
+						while (*s && (nsvg__isspace(*s) || *s == '%' || *s == ',')) s++;
+						if (*s) {
+							s = nsvg__parseNumber(s, buf, 64);
+							p->viewHeight = nsvg__atof(buf);
+						}
+					}
+				}
 			} else if (strcmp(attr[i], "preserveAspectRatio") == 0) {
 				if (strstr(attr[i + 1], "none") != 0) {
 					// No uniform scaling

[zezic]

@wcout do you have a time to create PR from this patch?

[poke1024]

This issue could get closed now I guess.

[Albrecht-S]

This issue could get closed now I guess.

@memononen For your information (and for all interested users):

Unfortunately commit c3ad36ef81992ff714cdbb4543cd67cb66daad8c, merged in PR #205 (commit 214cf85efcdc67524335ad0e2a2d5982246b6a72) on Apr 7, 2022 introduced a new locale dependency by using sscanf() with %f conversion specifier. Diff:

-	if (sscanf(str, "rgb(%u%%, %u%%, %u%%)", &r, &g, &b) == 3)	// decimal integer percentage
-		return NSVG_RGB(r*255/100, g*255/100, b*255/100);
+	float rf=0, gf=0, bf=0;
+	if (sscanf(str, "rgb(%f%%, %f%%, %f%%)", &rf, &gf, &bf) == 3)	// decimal integer percentage
+		return NSVG_RGB(round(rf*2.55f), round(gf*2.55f), round(bf*2.55f)); // (255 / 100.0f)

Before this commit only integer percent values would be parsed correctly, now fractional percent values are allowed but the locale dependency would break parsing fractional values if used on a locale that doesn't use '.' as separator.

(This is untested: noticed by code review.)

[oehhar]

Yes... This old issues. xfig was already sensible to this, that files could not exchanged, as German locale uses the "," as separator, but English locale uses ".". It would be great to also use "nsvg__atof(str)" for this case.

[Albrecht-S]

It would be great to also use "nsvg__atof(str)" for this case.

@oehhar I'm working on a fix and making good progress. I'll open a PR when all my final tests succeeded.

[Albrecht-S]

Please see PR #220 for my proposed fix.

[oehhar]

Thanks, Albrecht, great!

I suppose this may be closed.

As a side note: I use an archaic MS-VC6++ compiler for tests and it throws the following 4 warnings:

Warning 1

C:\test\git\tksvg\win\..\generic\nanosvg.h(1317) : warning C4244: '=' : conversion from 'double ' to 'float ', possible loss of data

The related code line is: rgbf[i] = nsvg__atof(str);

Warning 2

C:\test\git\tksvg\win\..\generic\nanosvg.h(1341) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data

The related code line is: rgbi[0] = roundf(rgbf[0] * 2.55f);

Warning 3

C:\test\git\tksvg\win\..\generic\nanosvg.h(1342) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data

The related code line is: rgbi[1] = roundf(rgbf[1] * 2.55f);

Warning 4

C:\test\git\tksvg\win\..\generic\nanosvg.h(1343) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data

The related code line is: rgbi[2] = roundf(rgbf[2] * 2.55f);

It might be ok (or not) to address them, as the rest of nanosvg does not throw any warnings. Simple casting may help.

Thank you all, Harald

[Albrecht-S]

@oehhar Thanks for your comment. Yes, MSVC compilers tend to issue all sorts of these warnings. Which warning level (usually /W3 or /W4) are you using?

That said, although I'm not the maintainer of this project, I intend to create another PR that should remove some kind of code duplication by implementing nsvg__strtof() and using it rather than the current nsvg__atof() as discussed elsewhere (https://github.com/memononen/nanosvg/pull/220#issuecomment-1179596094) in this project. I'll keep this issue in mind and I'll try to fix it with the new PR - unless someone else does it.

Unfortunately I'm currently too busy with another project but I'll come back to this when time permits.

[oehhar]

Dear Albrecht, thank you for the comment. There is no hurry. It is all great work and there is no need for beautification.

Your fix is in tksvg and tk 8.7 now, thanks for that.

About the warning level? It is "/W 3".

For your information, the whole nmake output is below. The compiler is MSVC6 with Paltform SDK 2003SP1.

Thank you again, Harald

C:\test\git\tksvg\win>nmake -f makefile.vc TCLDIR=c:\test\tcl8.6.12 TKDIR=c:\test\tk8.6.12 INSTALLDIR=c:\test\tksvg

Microsoft (R) Program Maintenance Utility   Version 6.00.9782.0
Copyright (C) Microsoft Corp 1988-1998. All rights reserved.

*** Using c:\test\tcl8.6.12\win\rules.vc
*** Compiler has 'Pentium 0x0f fix'

C:\test\git\tksvg\win>echo 0,11,0,0  1>>versions.vc
*** Building against Tcl at 'c:\test\tcl8.6.12'
*** Building against Tk at 'c:\test\tk8.6.12'
*** Intermediate directory will be 'C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic'
*** Output directory will be 'C:\test\git\tksvg\win\Release'
*** Installation, if selected, will be in 'c:\test\tksvg'
*** Suffix for binaries will be 't'
*** Compiler version 6. Target IX86, host AMD64.
  0 '@PACKAGE_VERSION@' => '0.11'
  1 '@PACKAGE_NAME@' => 'tksvg'
  2 '@PACKAGE_TCLNAME@' => 'tksvg'
  3 '@PKG_LIB_FILE@' => 'tksvg011t.dll'
  4 '@PKG_LIB_FILE8@' => 'tksvg011t.dll'
  5 '@PKG_LIB_FILE9@' => 'tcl9tksvg011t.dll'
        rc -fo C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\tksvg.res -r -i "C:\test\git\tksvg\win\..\generic" -i "C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic"  -I"c:\test\tcl8.6.12\generic" -I"c:\test\tcl8.6.12\win"  /DDEBUG=0 -d UNCHECKED=0  /DCOMMAVERSION=0,11,0,0  /DDOTVERSION=\"0.11\"  /DVERSION=\"011\"  /DSUFX=\"t\"  /DPROJECT=\"tksvg\"  /DPRJLIBNAME=\"tksvg011t.dll\" C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\tksvg.rc
        cl -nologo -c /D_ATL_XP_TARGETING  -W3 -FpC:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\ -Op -QI0f -O2 -YX -MD -I"c:\test\tcl8.6.12\generic" -I"c:\test\tcl8.6.12\win" -I"c:\test\tk8.6.12\generic" -I"c:\test\tk8.6.12\win" -I"c:\test\tk8.6.12\xlib"  -I"C:\test\git\tksvg\win\..\generic" -I"C:\test\git\tksvg\win\..\win" -I"C:\test\git\tksvg\win\..\compat"  -Dinline=__inline /DSTDC_HEADERS /DUSE_NMAKE=1 /DMP_NO_STDINT=1 /DTCL_THREADS=1 /DUSE_THREAD_ALLOC=1 /DNDEBUG /DTCL_CFG_OPTIMIZED /DNO_STRTOI64=1 /DUSE_TCL_STUBS /DUSE_TCLOO_STUBS /DUSE_TK_STUBS /DPACKAGE_NAME="\"tksvg\""  /DPACKAGE_TCLNAME="\"tksvg\""  /DPACKAGE_VERSION="\"0.11\""  /DMODULE_SCOPE=extern /DBUILD_tksvg -FoC:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\ @C:\Users\oehhar\AppData\Local\Temp\nmc06916.
tkImgSVG.c
C:\test\git\tksvg\win\..\generic\nanosvg.h(1317) : warning C4244: '=' : conversion from 'double ' to 'float ', possible loss of data
C:\test\git\tksvg\win\..\generic\nanosvg.h(1341) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data
C:\test\git\tksvg\win\..\generic\nanosvg.h(1342) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data
C:\test\git\tksvg\win\..\generic\nanosvg.h(1343) : warning C4244: '=' : conversion from 'float ' to 'unsigned int ', possible loss of data
        link -nologo -machine:IX86  -release -opt:ref -opt:icf,3 -dll -out:C:\test\git\tksvg\win\Release\tksvg011t.dll kernel32.lib advapi32.lib gdi32.lib user32.lib uxtheme.lib  "c:\test\tcl8.6.12\win\Release\tclstub86.lib" "c:\test\tcl8.6.12\win\Release\tcl86t.lib" "c:\test\tk8.6.12\win\Release\tkstub86.lib" "c:\test\tk8.6.12\win\Release\tk86t.lib" C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\tkImgSVG.obj C:\test\git\tksvg\win\Release\tksvg_ThreadedDynamic\tksvg.res
   Creating library C:\test\git\tksvg\win\Release\tksvg011t.lib and object C:\test\git\tksvg\win\Release\tksvg011t.exp