GNU bug report logs - #75712
HAVE_RSVG: svg_css_length_to_pixels doesn't handle RSVG_UNIT_CH

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 75712 <at> debbugs.gnu.org, Stefan Kangas <stefankangas <at> gmail.com>
Subject: bug#75712: HAVE_RSVG: svg_css_length_to_pixels doesn't handle RSVG_UNIT_CH
Date: Tue, 21 Jan 2025 15:04:33 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Cc: Pip Cet <pipcet <at> protonmail.com>
>> From: Stefan Kangas <stefankangas <at> gmail.com>
>> Date: Mon, 20 Jan 2025 20:25:18 -0600
>>
>> --- a/src/image.c
>> +++ b/src/image.c
>> @@ -12008,9 +12008,15 @@ svg_css_length_to_pixels (RsvgLength length, double dpi, int font_size)
>>  	 what size we make the image.  */
>>        value = 0;
>>        break;
>> -    default:
>> -      /* We should never reach this.  */
>> -      value = 0;
>> +#if LIBRSVG_CHECK_VERSION (2, 58, 0)
>> +    case RSVG_UNIT_CH:
>> +      /* FIXME: With CSS 3, "the ch unit falls back to 0.5em in the
>> +	 general case, and to 1em when it would be typeset upright".
>> +	 However, I could not find a way to easily get the relevant CSS
>> +	 attributes using librsvg.  Thus, we simply wrongly assume the
>> +	 general case is always true here.  See Bug#75712.  */
>> +      value = value * font_size / 2.0;
>> +#endif
>
> This makes the code worse for librsvg versions before 2.58.  Why not
> keep the old code for older versions?

If the librsvg library and the librsvg headers are both old, this code
is never reached.

If the headers were old but the library has been updated, I agree that
the change in behavior is incorrect.  Returning 0 means we can't do
anything with the unit, which is true.

Here's a proposed patch which:
1. prints a warning about this situation (in case the library changes)
2. omits the default label so GCC also warns, even without -Wswitch-enum
(in case the headers change)
3. includes too much commentary rather than too little.

diff --git a/src/image.c b/src/image.c
index b8405d81111..28d55a575ee 100644
--- a/src/image.c
+++ b/src/image.c
@@ -11965,34 +11965,27 @@ svg_css_length_to_pixels (RsvgLength length, double dpi, int font_size)
     {
     case RSVG_UNIT_PX:
       /* Already a pixel value.  */
-      break;
+      return value;
     case RSVG_UNIT_CM:
       /* 2.54 cm in an inch.  */
-      value = dpi * value / 2.54;
-      break;
+      return dpi * value / 2.54;
     case RSVG_UNIT_MM:
       /* 25.4 mm in an inch.  */
-      value = dpi * value / 25.4;
-      break;
+      return dpi * value / 25.4;
     case RSVG_UNIT_PT:
       /* 72 points in an inch.  */
-      value = dpi * value / 72;
-      break;
+      return dpi * value / 72;
     case RSVG_UNIT_PC:
       /* 6 picas in an inch.  */
-      value = dpi * value / 6;
-      break;
+      return dpi * value / 6;
     case RSVG_UNIT_IN:
-      value *= dpi;
-      break;
+      return value * dpi;
     case RSVG_UNIT_EM:
-      value *= font_size;
-      break;
+      return value * font_size;
     case RSVG_UNIT_EX:
       /* librsvg uses an ex height of half the em height, so we match
 	 that here.  */
-      value = value * font_size / 2.0;
-      break;
+      return value * font_size / 2.0;
     case RSVG_UNIT_PERCENT:
       /* Percent is a ratio of the containing "viewport".  We don't
 	 have a viewport, as such, as we try to draw the image to it's
@@ -12006,14 +11999,27 @@ svg_css_length_to_pixels (RsvgLength length, double dpi, int font_size)
 	 spec, this will work out correctly as librsvg will still
 	 honor the percentage sizes in its final rendering no matter
 	 what size we make the image.  */
-      value = 0;
-      break;
-    default:
-      /* We should never reach this.  */
-      value = 0;
-    }
-
-  return value;
+      return 0;
+#if LIBRSVG_CHECK_VERSION (2, 58, 0)
+    case RSVG_UNIT_CH:
+      /* FIXME: With CSS 3, "the ch unit falls back to 0.5em in the
+	 general case, and to 1em when it would be typeset upright".
+	 However, I could not find a way to easily get the relevant CSS
+	 attributes using librsvg.  Thus, we simply wrongly assume the
+	 general case is always true here.  See Bug#75712.  */
+      return value * font_size / 2.0;
+#endif
+    }
+
+  /* The rsvg header files say that more values may be added to this
+     enum, but there doesn't appear to be a way to get a string
+     representation of the new enum value.  The unfortunate
+     consequence is that the only thing we can do is to report the
+     numeric value.  */
+  image_error ("Unknown RSVG unit, code: %d", (int) length.unit);
+  /* Return 0; this special value indicates that another method of
+     obtaining the image size must be used.  */
+  return 0;
 }
 #endif
 
> Also, pointing to bug#75712 is not useful here: this bug had accrued
> too many completely unrelated messages, so anyone who'd like to have

Wrong bug.  This is bug#75712.

Pip

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.