Bug 621699 - make librsvg gio friendly
make librsvg gio friendly
Status: RESOLVED FIXED
Product: librsvg
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: librsvg maintainers
librsvg maintainers
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-06-15 21:12 UTC by Christian Persch
Modified: 2010-06-22 16:02 UTC (History)
2 users (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add GIO convenience to librsvg (17.39 KB, patch)
2010-06-15 21:12 UTC, Christian Persch
accepted-commit_now Details | Diff | Review
Use GConverter for gzip support (10.46 KB, patch)
2010-06-15 22:00 UTC, Christian Persch
none Details | Diff | Review
My patch (6.15 KB, patch)
2010-06-16 11:42 UTC, Hiroyuki Ikezoe
none Details | Diff | Review

Description Christian Persch 2010-06-15 21:12:10 UTC
librsvg should have gio convenience functions, e.g. to read data from a GInputStream (instead of having to do that yourself via rsvg_handle_write + _close), and to create a handle from GFile, or GInputStream.

This will also make it easy to support .svg.gz loading by just using a GConverterInputStream later.

I also think we should just make gio mandatory, instead of optional in configure.

Proposed patch attached.
Comment 1 Christian Persch 2010-06-15 21:12:40 UTC
Created attachment 163729 [details] [review]
Add GIO convenience to librsvg

Adds rsvg_handle_read_stream() to read the handle's data from a
GInputStream, and rsvg_handle_new_from_{gfile,stream} convenience
functions analogous to rsvg_handle_new_from_{file,data}.
Comment 2 Christian Persch 2010-06-15 22:00:04 UTC
Created attachment 163742 [details] [review]
Use GConverter for gzip support

Supporting gzipped input is now easy! Remove libgsf, and just use
GZlibDecompressor on gio >= 2.24.0.

Use a memory input stream to store the data from rsvg_handle_write(),
and process all the data in rsvg_handle_close(). This may or may not
constitute a behaviour change; not sure.
Comment 3 Hiroyuki Ikezoe 2010-06-16 11:28:02 UTC
Wow! I like these patches very much!

Please commit and let's fix any issues in git master.
Comment 4 Hiroyuki Ikezoe 2010-06-16 11:41:17 UTC
Review of attachment 163742 [details] [review]:

I did a similar work last night.

In my patch, each gzipped data chunk is converted in rsvg_handle_write(). I prefer my approach since there is no need to allocate extra memory. Can you change this patch to such approach?

::: rsvg-base.c
@@ +1742,3 @@
+        converter = G_CONVERTER (g_zlib_decompressor_new (G_ZLIB_COMPRESSOR_FORMAT_GZIP));
+        new_input_stream = g_converter_input_stream_new (priv->data_input_stream,
+                                                         converter);

This converter object is leaking.
Comment 5 Hiroyuki Ikezoe 2010-06-16 11:42:27 UTC
Created attachment 163804 [details] [review]
My patch

For the reference I attach the patch.
Comment 6 Hiroyuki Ikezoe 2010-06-16 11:53:22 UTC
I will be off-line by Sunday, so please feel free to go ahead.
Comment 7 Christian Persch 2010-06-16 13:38:41 UTC
I changed the code back to the way it was before with libgsf: _write with gzipped data stores the data, then processes all-at-once in _close, while non-gzipped data is processed in the chunks as passed to _write. So there shouldn't be any behaviour change, anymore.

About your patch: that might be a good idea since it saves memory, but I'm not sure it works like that; don't you have to check to G_ERROR_NO_SPACE and provide a bigger output buffer in this case? Anyway, that can be improved later.
Comment 8 Christian Persch 2010-06-16 13:43:48 UTC
Also, should I make gio required, or add some #if GLIB_CHECK_VERSION (2, 16, 0) to the rsvg.h header around the new functions?
Comment 9 Hiroyuki Ikezoe 2010-06-22 11:24:13 UTC
I think depending GIO is reasonable, but GConverter part should be encolosed by GLIB_CHECK_VERSION because GConvert is a brand new feature in GIO.
Comment 10 Christian Persch 2010-06-22 15:56:02 UTC
Fixed on master.

I made gio required, but kept the libgsf code for when gio version is < 2.24.
Comment 11 Christian Persch 2010-06-22 16:02:43 UTC
I also created a gnome-2-30 branch that doesn't contain these recent changes.

Note You need to log in before you can comment on or make changes to this bug.