Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial SVG backend #112

Merged
merged 1 commit into from
Jan 21, 2020
Merged

Partial SVG backend #112

merged 1 commit into from
Jan 21, 2020

Conversation

Ralith
Copy link
Contributor

@Ralith Ralith commented Nov 24, 2019

Lightly tested; produces output similar to the cairo backend for test images 1, 3 and 4. Attempts to use text and images will produce NotSupported error; the unimplemented invocations are all statically unreachable as a result.

For some reason I couldn't get SVG line elements to display, but paths work fine and are more concise anyway.

@Ralith
Copy link
Contributor Author

Ralith commented Nov 25, 2019

Implementing TextLayout faithfully would present some challenges because we're not actually rendering anything. At minimum, I think we'd need to pull in a text shaping engine (perhaps behind a feature flag) and disclaim any prospect of correctness if the document isn't displayed using exactly the same fonts that were specified when generating the file.

Images present a different challenge: we can embed base64'd png or jpeg (or other?) data, but calling out to an image encoder when someone calls make_image is pretty weird. This could also be behind a feature flag, and configured (to e.g. select codec and compression level) via inherent methods.

piet-svg/src/lib.rs Outdated Show resolved Hide resolved
piet-svg/src/lib.rs Outdated Show resolved Hide resolved
piet-svg/src/lib.rs Outdated Show resolved Hide resolved
@Ralith
Copy link
Contributor Author

Ralith commented Dec 13, 2019

A better solution to the text issue might be to render text as paths, for which there at least exists an authoritative answer to the queries. Shame not to have working copy/paste though.

Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this looks like a good start. I'm ok with this going in unfinished, but would like at least a short docstring at the module level explaining that, and at least a one line docstring for all public interfaces.

From what I can tell, images are largely a function of resource linking. But these could be done as base64 data uri's?

Text is going to be difficult. I'm not a fan of flattening to paths, as I think a large part of the value of SVG is preserving some structure. Longer term, we should probably have some plan for working with real fonts, but they present at least as many problems as images.

@@ -0,0 +1,18 @@
//! Basic example of rendering to a SVG
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming doesn't follow the pattern, this should be "basic-svg.rs". That said, I agree the naming scheme is not ideal, and the d2d one also doesn't follow the pattern.

@Ralith
Copy link
Contributor Author

Ralith commented Dec 17, 2019

Thanks for the review!

From what I can tell, images are largely a function of resource linking. But these could be done as base64 data uri's?

We can easily add backend-specific methods for linking external resources, and probably that's what we should direct users towards here. My concern is mainly about implementing make_image, which consumes raw bitmap data. Pulling in an image codec just to support a really ugly and inefficient embedding is unfortunate, and hardwiring any one codec would tend to give bad results for many inputs. Is there maybe a way to encode raw bitmap data in a data URI directly?

@RazrFalcon
Copy link

Is there maybe a way to encode raw bitmap data in a data URI directly?

AFAIK, SVG allows only image/png, image/jpeg and image/svg+xml.

@raphlinus
Copy link
Contributor

It's a good question. It's certainly possible to generate a PNG without actually doing compression, so if we really didn't want to depend on any other crates, it's possible. But I think perhaps the best thing to do here is pull in the png crate - that crate alone is not a super heavy dependency.

It's starting to seem like there might need to be a more general mechanism for resources that are not generated through factory methods on the render context. I can see that coming into play, for example, for images that are generated gpu-side, for the other backends. I'll continue to think about this.

@Ralith
Copy link
Contributor Author

Ralith commented Dec 30, 2019

Added (very) brief documentation and renamed the example, and tweaked types for better forwards-compatibility.

@cmyr
Copy link
Member

cmyr commented Jan 21, 2020

What's the status of this?

@Ralith
Copy link
Contributor Author

Ralith commented Jan 21, 2020

I think this is ready to go.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, what was the original motivation for this?

I have one little nit about version numbers but otherwise I'm happy. It might be good to have an issue outlining the work that's missing?

piet-svg/Cargo.toml Outdated Show resolved Hide resolved
@Ralith
Copy link
Contributor Author

Ralith commented Jan 21, 2020

Just out of curiosity, what was the original motivation for this?

My current use of druid is a 2D vector graphics toy which I want to support both interactive editing and export. Using the same drawing code for both is extremely attractive, for obvious reasons.

spiral

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! :)

@cmyr cmyr merged commit 18c6441 into linebender:master Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants