Better support for PDF, SVG and PostScript. #165

meh · 2017-11-11T16:53:28Z

Don't merge yet, just need some eyes on the unsafe part.

meh · 2017-11-12T15:15:21Z

@GuillaumeGomez this is ready as far as I'm concerned.

EPashkin · 2017-11-12T16:03:35Z

src/lib.rs

@@ -125,9 +122,19 @@ mod rectangle;
 mod region;
 mod surface;
 mod matrices;
+mod support;


Seems this better have #[cfg(any(feature = "pdf", feature = "svg", feature = "ps", feature = "dox"))]

Good point, commit incoming.

EPashkin · 2017-11-12T16:16:05Z

Second problem that all docs need moved to https://github.com/gtk-rs/lgpl-docs repo to cairo/docs.rs.
Current comments can be stripped with rustdoc-stripper binary from https://github.com/GuillaumeGomez/rustdoc-stripper

EPashkin · 2017-11-12T16:20:20Z

@GuillaumeGomez there problem with regenerating stripped comments: in ps.rs comment moved from impl<'a> Stream<'a>::new to impl<'a> Writer<'a>::new

meh · 2017-11-12T16:26:45Z

@EPashkin any direction to move them over without suffering too much?

EPashkin · 2017-11-12T16:29:51Z

@meh Theoretically you just need run rustdoc-stripper on cairo repo, it produces comments.md that need added to doc.rs.
But as it not restored rightly better wait @GuillaumeGomez answer.

meh · 2017-11-12T16:34:02Z

@EPashkin oh I see, yeah I'll wait, he's probably still traveling 🐼

GuillaumeGomez · 2017-11-12T19:20:11Z

Ok so, if docs issue you have, regen the docs (cargo build --features "embed-lgpl" is my memory is correct) then add the new docs then strip everything in the comment file and send us the comment file.

If it wasn't the problem, then it just mean I need to sleep and I'll check this tomorrow. 😀

EPashkin · 2017-11-12T19:56:57Z

@GuillaumeGomez for tomorrow check:
I run last version of rustdoc-stripper for this PR,
get filled comments.md,
on call rustdoc-stripper -g it restored all comments with one exception:
in ps.rs comment moved from impl<'a> Stream<'a>::new to impl<'a> Writer<'a>::new

GuillaumeGomez · 2017-11-12T20:05:50Z

That's weird. I suppose it's a rustdoc-stripper's issue but I don't see what it is for the moment. I'll look into it.

EPashkin · 2017-11-12T20:11:49Z

@meh, @GuillaumeGomez Found "solution": it restored right way if Writer::new also have comments

GuillaumeGomez · 2017-11-12T20:13:56Z

That's weird...

meh · 2017-11-12T21:04:30Z

Should I also purge the documentation from the git history?

GuillaumeGomez · 2017-11-12T21:07:58Z

It depends if it's lgpl docs or not.

meh · 2017-11-12T23:06:27Z

I don't know, I wrote them 🤔

GuillaumeGomez · 2017-11-13T08:49:01Z

Then it's not lgpl. :)

meh · 2017-11-13T13:34:45Z

I think I did something wrong while generating the docs, see pull request on docs.

sdroege · 2017-11-15T14:43:12Z

cairo-sys-rs/src/enums.rs

+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum PdfVersion {
+    _1_4,
+    _1_5,


See gtk-rs/gir#480

What do I need to do exactly here? Kind of unclear from the issue.

Should I just add an Unknown variant?

In the -sys crate you use an integer and constants, in the non-sys crate you have a real enum with the Unknown variant

Is this still the case?

sdroege · 2017-11-15T14:45:50Z

src/ps.rs

+#[cfg(feature = "use_glib")]
+use glib::translate::*;
+
+pub struct File {


I see some potential of sharing code here (macro I guess?) between PDF and PS. It's basically the same except for the types and the functions called.

Yeah, there's a lot of code duplication between PDF, PS and SVG, I could add a macro to src/support.rs that generates those, wasn't sure if that was wanted or not.

sdroege · 2017-11-15T14:48:54Z

src/support.rs

+                unsafe {
+                    let mut out = Box::from_raw(buffer);
+                    out.extend(data);
+                    Box::into_raw(out);


This seems dangerous. The buffer you store in Buffer could become invalid in theory. Box::from_raw() followed by Box::into_raw() could in theory move things to a different place on the heap.

I'm pretty sure that's safe, or stuff like owning_ref and rental would explode in really funny ways.

The part I'm unsure about is the story with aliasing which might be UB there.

The problem with from_raw/into_raw() is that they operate on owned values. They could do whatever they want with them, including moving to different memory locations. The assumption is that nothing is borrowing the data currently (and nothing is as far as the compiler is concerned, otherwise into_raw() would not compile), and with all your pointers that still point to the data you're on your own and need to ensure correctness.

I would recommend to work directly on the pointers here: let out = &mut *buffer;

I'll look into it.

sdroege · 2018-08-06T06:56:00Z

What's the status/plan here?

GuillaumeGomez · 2018-08-06T08:04:05Z

I'll see @meh this week-end. I'll try to motivate him to finish it. :)

meh · 2018-08-06T10:33:12Z

It will happen, I believe.

GuillaumeGomez · 2018-11-15T15:43:47Z

🎉 Finally! 🎉

Thanks!

yannleretaille · 2018-11-15T17:14:06Z

🎉

ekg · 2019-04-26T17:54:34Z

The Cairo library has no such concepts in its PDF, SVG, PS,... support, it just has a single object for a surface representing each format which can be constructed in various ways. Since this is a binding library, I think it's bets to stay true to the way things are organized in the original library, otherwise it gets confusing for users.

This is absolutely true. I am going to have to look for a simpler set of bindings :(

meh mentioned this pull request Nov 11, 2017

Add T: io::Write support to PDFSurface #154

Closed

meh force-pushed the everything branch 4 times, most recently from 30aa165 to acf1a4b Compare November 12, 2017 14:54

EPashkin reviewed Nov 12, 2017

View reviewed changes

GuillaumeGomez mentioned this pull request Nov 12, 2017

Change CallbackGuard to require explicit destruction gtk-rs/glib#250

Closed

EPashkin mentioned this pull request Nov 13, 2017

Update documentation for cairo gtk-rs/lgpl-docs#37

Merged

sdroege reviewed Nov 15, 2017

View reviewed changes

Add support for SVG, PS and PDF surfaces

d39b674

meh force-pushed the everything branch from d595c20 to d39b674 Compare November 15, 2018 12:25

GuillaumeGomez merged commit d00687e into gtk-rs:master Nov 15, 2018

Better support for PDF, SVG and PostScript. #165

Better support for PDF, SVG and PostScript. #165

Conversation

meh commented Nov 11, 2017

meh commented Nov 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EPashkin commented Nov 12, 2017

EPashkin commented Nov 12, 2017

meh commented Nov 12, 2017

EPashkin commented Nov 12, 2017

meh commented Nov 12, 2017

GuillaumeGomez commented Nov 12, 2017

EPashkin commented Nov 12, 2017

GuillaumeGomez commented Nov 12, 2017

EPashkin commented Nov 12, 2017

GuillaumeGomez commented Nov 12, 2017

meh commented Nov 12, 2017

GuillaumeGomez commented Nov 12, 2017

meh commented Nov 12, 2017

GuillaumeGomez commented Nov 13, 2017

meh commented Nov 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdroege Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdroege commented Aug 6, 2018

GuillaumeGomez commented Aug 6, 2018

meh commented Aug 6, 2018

GuillaumeGomez commented Nov 15, 2018

yannleretaille commented Nov 15, 2018

ekg commented Apr 26, 2019

sdroege Nov 15, 2017 •

edited

Loading