fontforge / libspiro

Spiro is the creation of Raph Levien. It simplifies the drawing of beautiful curves. (Migrated here from libspiro.sourceforge.net on 2013-04-20)
GNU General Public License v3.0
107 stars 25 forks source link

Rust port #27

Closed ctrlcctrlv closed 2 years ago

ctrlcctrlv commented 4 years ago

Hello Joe:

As part of my new font editor project (currently only a glyph editor :wink:), I created a Spiro binding in Rust.

https://github.com/mfeq/spiro-sys

I'll also be making what Rust users call a "safe" version; basically, this is a version which wraps the memory-unsafe functions of libspiro to provide a (more) memory-safe API.

I noticed while doing this that the README's drawing of path 5 seems wrong. I got this instead:

Could very well be my fault, but wanted to ask you.

The code that generates that is here:

https://github.com/mfeq/spiro-sys/blob/22b9e1e9e85f4471910876cf4232edec4dbd8441/tests/spiro_to_beziers.rs#L27-L57

JoesCat commented 4 years ago

Actually, using FontForge X,Y cooridinates as seen in the FontForge drawing window, the Y dimension goes positive value up, so it looks like you need to make negative Y to do the same for fonts. However, you did catch a bug. Good catch.

The libspiro drawing example in the readme is a fabricated mixture from the FontForge GUI using the '[' and ']' commands, and some expected bezier target points if FontForge were able to handle 'a' and 'h', which doesn't exist (yet) at this time. At the time I created the drawing, the original curve was:

move from 0,0 to 100,100 then...
    {100, 100, '{'}, <-following test5 in verbose mode, this is seen as 'c'
    {200, 200, '['},
    {300, 200, ']'},
    {400, 150, 'c'},
    {300, 100, '['},
    {200, 100, ']'},
    {150,  50, 'c'},
    {100,   0, '['},
    {  0,-100, ']'},
    {-50,-200, 'c'},
    {-80,-250, '}'} /* test 23 reverses this list. */

If you draw your curve starting from the 100,100, it should match the drawing example.

The libspiro path5 test needs to be fixed to exclude 0,0 as the start, or the drawing needs to be corrected to actually include 0,0

Yes - it's a drawing bug.

JoesCat commented 4 years ago

TODO - Will need to redraw images starting from 0,0

ctrlcctrlv commented 4 years ago

Moving discussion from #28 here.

I'm still not exactly sure what I've done to upset you ("FYI - if you want others to respect your license, then you should respect other licenses too"), but I managed to get @raphlinus 's code to compile:

% osboxes@osboxes:~/Workspace/raphlevien/spiro/ppedit$ ./spiro 
100 800 translate 1 -1 scale 1 setlinewidth
334 117 moveto
% ks = [ 0 0 0 0 ]
305 176 lineto
% ks = [ -0.721873 0 0 0 ]
279.449 153.974 245.733 141.648 212 142 curveto
% ks = [ -1.25088 -1.5795 0 0 ]
201.226 142.112 190.267 143.529 180.511 148.103 curveto
175.633 150.39 171.099 153.461 167.348 157.327 curveto
163.596 161.194 160.641 165.869 159 171 curveto
% ks = [ -1.58436 3.38715 0 0 ]
156.364 179.243 157.251 188.403 160.654 196.36 curveto
164.057 204.316 169.853 211.112 176.664 216.45 curveto
190.287 227.126 207.444 231.953 224 237 curveto
% ks = [ 1.05213 1.75697 0 0 ]
250.322 245.024 276.649 254.369 299.324 269.96 curveto
321.998 285.551 340.921 308.162 347 335 curveto
% ks = [ 1.81724 -1.65783 0 0 ]
351.166 353.392 349.066 373.009 341.949 390.473 curveto
334.833 407.936 322.824 423.251 308.134 435.075 curveto
278.752 458.724 239.706 467.893 202 467 curveto
% ks = [ 0.561231 0 0 0 ]
159.176 465.986 116.715 452.651 81 429 curveto
% ks = [ 0 0 0 0 ]
114 368 lineto
% ks = [ -0.412425 0 0 0 ]
140.251 385.152 170.075 396.808 201 402 curveto
% ks = [ -1.56807 -2.33561 0 0 ]
216.006 404.519 231.742 405.476 246.158 400.607 curveto
253.366 398.172 260.137 394.269 265.488 388.861 curveto
270.839 383.452 274.72 376.5 276 369 curveto
% ks = [ -1.3892 2.72274 0 0 ]
277.287 361.46 275.914 353.582 272.701 346.641 curveto
269.488 339.699 264.498 333.665 258.672 328.709 curveto
247.02 318.796 232.367 313.255 218 308 curveto
% ks = [ 0.937503 1.97582 0 0 ]
191.978 298.481 165.57 289.162 142.279 274.152 curveto
118.988 259.143 98.7273 237.609 91 211 curveto
% ks = [ 1.15736 -0.266166 0 0 ]
85.821 193.166 86.5807 173.749 92.5866 156.176 curveto
98.5925 138.603 109.749 122.908 124 111 curveto
% ks = [ 0.765271 -0.522263 0 0 ]
138.28 99.0677 155.539 90.9339 173.578 86.3634 curveto
191.617 81.7928 210.435 80.7131 229 82 curveto
% ks = [ 0.505082 0 0 0 ]
266.199 84.5786 302.694 96.7435 334 117 curveto
stroke
showpage

Only changed spiro.c:

diff --git a/ppedit/spiro.c b/ppedit/spiro.c
index ba5035a..f7de155 100644
--- a/ppedit/spiro.c
+++ b/ppedit/spiro.c
@@ -890,7 +890,6 @@ get_knot_th(const spiro_seg *s, int i)
     }
 }

-#ifdef UNIT_TEST
 #include <stdio.h>
 #include <sys/time.h> /* for gettimeofday */

@@ -1058,4 +1057,3 @@ int main(int argc, char **argv)
 {
     return test_curve();
 }
-#endif

And a new quick and dirty Makefile:

CFLAGS = -g -Wall
LDFLAGS = -g 
LDLIBS = -lm

all:    spiro

spiro: bezctx.o bezctx_hittest.o plate.o sexp.o bezctx_ps.o spiro.o
ctrlcctrlv commented 4 years ago

Well now that I've done all this work to get a build...I've managed to transpile it with c2rust, so maybe I'm just going to go with a pure Rust implementation and get out of your hair.

I simplified the Makefile (pared it down to bezctx.o spiro.o) and this worked:

osboxes@osboxes:~/Workspace/raphlevien/spiro/ppedit$ intercept-build make
cc -g -Wall   -c -o spiro.o spiro.c
spiro.c: In function ‘spiro_seg_to_bpath’:
spiro.c:792:15: warning: logical not is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
  792 |     if (!bend > 1e-8) {
      |               ^
spiro.c:792:9: note: add parentheses around left hand side expression to silence this warning
  792 |     if (!bend > 1e-8) {
      |         ^~~~~
      |         (    )
spiro.c: In function ‘test_integ’:
spiro.c:912:16: warning: variable ‘th’ set but not used [-Wunused-but-set-variable]
  912 |     double ch, th;
      |                ^~
spiro.c:912:12: warning: variable ‘ch’ set but not used [-Wunused-but-set-variable]
  912 |     double ch, th;
      |            ^~
cc -g -Wall   -c -o bezctx.o bezctx.c
cc -g   spiro.o bezctx.o  -lm -o spiro
osboxes@osboxes:~/Workspace/raphlevien/spiro/ppedit$ ../../../c2rust/target/debug/c2rust transpile compile_commands.json 
spiro.c:792:9: warning: logical not is only applied to the left hand side of this comparison [-Wlogical-not-parentheses]
    if (!bend > 1e-8) {
        ^     ~
spiro.c:792:9: note: add parentheses after the '!' to evaluate the comparison first
    if (!bend > 1e-8) {
        ^
         (          )
spiro.c:792:9: note: add parentheses around left hand side expression to silence this warning
    if (!bend > 1e-8) {
        ^
        (    )
1 warning generated.
Transpiling spiro.c
Transpiling bezctx.c

Here are the transpiled files with @immunant's project:

https://gist.github.com/ctrlcctrlv/82042e64a27753fa69a0ffbb0cc64fbe

Cool stuff if you ask me. Let me figure out if it actually builds.

ctrlcctrlv commented 4 years ago

Dude, whoa 🤯

osboxes@osboxes:~/Workspace/raphspiro/src$ cargo run
    Updating crates.io index
   Compiling libc v0.2.77
   Compiling raphspiro v0.1.0 (/home/osboxes/Workspace/raphspiro)
warning: variable `ch` is assigned to, but never used
    --> src/main.rs:1074:9
     |
1074 |     let mut ch: libc::c_double = 0.;
     |         ^^^^^^
     |
     = note: `#[warn(unused_variables)]` on by default
     = note: consider using `_ch` instead

warning: variable `th` is assigned to, but never used
    --> src/main.rs:1075:9
     |
1075 |     let mut th: libc::c_double = 0.;
     |         ^^^^^^
     |
     = note: consider using `_th` instead

warning: unused variable: `argc`
    --> src/main.rs:1399:18
     |
1399 | unsafe fn main_0(mut argc: libc::c_int, mut argv: *mut *mut libc::c_char)
     |                  ^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_argc`

warning: unused variable: `argv`
    --> src/main.rs:1399:41
     |
1399 | unsafe fn main_0(mut argc: libc::c_int, mut argv: *mut *mut libc::c_char)
     |                                         ^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_argv`

warning: 4 warnings emitted

    Finished dev [unoptimized + debuginfo] target(s) in 8.96s
     Running `/home/osboxes/Workspace/raphspiro/target/debug/raphspiro`
100 800 translate 1 -1 scale 1 setlinewidth
334 117 moveto
% ks = [ 0 0 0 0 ]
305 176 lineto
% ks = [ -0.721873 0 0 0 ]
279.449 153.974 245.733 141.648 212 142 curveto
...

Check it out @raphlinus , your 13 year old code running in pure Rust! :D

OK, well, now that it works real well I don't need to use my spiro-sys or bindgen or anything I don't think!

ctrlcctrlv commented 4 years ago

OK so Rustifying this isn't going to be so hard I don't think. After these it still builds:

:%s/libc::c_double/f64/g
:%s/libc::c_int/isize/g
:%s/libc::c_uint/usize/g

Far out 😎

JoesCat commented 2 years ago

Updates for path5.png and path6.png - done.

ctrlcctrlv commented 2 years ago

Thanks for the reminder of this issue. My Rust port has been done for a while. I call it spiro-inner. https://github.com/MFEK/spiro-inner.rlib

(Because the idea is, I will wrap it with an "outer" safe library. The "inner" library breaks several Rust conventions by using unsafe code even in pure Rust that doesn't call out to C. This is because it's a product of C2Rust that I heavily modified manually.)

See e.g., and compare it to function in this library:

https://github.com/MFEK/spiro-inner.rlib/blob/7d00cfd845c07fb5f12a085a9ad2c7bb4a38e0d8/src/lib.rs#L98-188

This issue is therefore solved...Rust port is done and has been done for a while.

Oh, and, Rust port is of @raphlinus' Apache2 code, not the current GPL code. I was very careful not to use any of the later work, spiro-inner is a straight port of the 2007 code, jettisoning the APIs added by George Williams and @JoesCat.

Meanwhile, spiro-sys is not a pure Rust project. It is a binding to a system installed libspiro, and does support the GWW/@joescat APIs.

ctrlcctrlv commented 1 year ago

The Rust port is much improved and uses no more unsafe code. I'm happy enough with it to call https://docs.rs/spiro/ as 1.0. This version is also based on @raphlinus' code only so is Apache v2.0 like the original.

Example of use:

use spiro::{BezCtxGpPenOpsData, BezierContext};
use glifparser::{Glif, outline::ToOutline as _};

let mut ctx = BezierContext::<BezCtxGpPenOpsData, ()>::new();
let path = test_data!();
ctx.run_spiro(&path);
let mut glif = Glif::<()>::new();
glif.outline = Some(ctx.data.ops_path.to_outline());
eprintln!("{}", glifparser::glif::write(&glif).unwrap());

Yielding:

<?xml version="1.0" encoding="UTF-8"?>
<glyph name="" format="2">
  <outline>
    <contour>
      <point x="229" y="82" type="curve"/>
      <point x="266.19907" y="84.57864"/>
      <point x="302.69357" y="96.74348"/>
      <point x="334" y="117" type="curve"/>
      <point x="334" y="117"/>
      <point x="305" y="176"/>
      <point x="305" y="176" type="curve"/>
      <point x="279.44864" y="153.9741"/>
      <point x="245.7326" y="141.64781"/>
      <point x="212" y="142" type="curve"/>
      <point x="201.22578" y="142.11249"/>
      <point x="190.2668" y="143.52855"/>
      <point x="180.51115" y="148.10278" type="curve"/>
      <point x="175.63333" y="150.38991"/>
      <point x="171.09901" y="153.46071"/>
      <point x="167.3476" y="157.32736" type="curve"/>
      <point x="163.59619" y="161.194"/>
      <point x="160.64076" y="165.86853"/>
      <point x="159" y="171" type="curve"/>
      <point x="156.3644" y="179.24281"/>
      <point x="157.25105" y="188.40298"/>
      <point x="160.65417" y="196.3597" type="curve"/>
      <point x="164.0573" y="204.31639"/>
      <point x="169.85254" y="211.11214"/>
      <point x="176.66406" y="216.45007" type="curve"/>
      <point x="190.2871" y="227.12595"/>
      <point x="207.44443" y="231.95279"/>
      <point x="224" y="237" type="curve"/>
      <point x="250.3216" y="245.02454"/>
      <point x="276.6491" y="254.36865"/>
      <point x="299.32373" y="269.9597" type="curve"/>
      <point x="321.99838" y="285.55072"/>
      <point x="340.92078" y="308.1623"/>
      <point x="347" y="335" type="curve"/>
      <point x="351.1662" y="353.39227"/>
      <point x="349.06635" y="373.00876"/>
      <point x="341.94952" y="390.47253" type="curve"/>
      <point x="334.8327" y="407.9363"/>
      <point x="322.82446" y="423.2509"/>
      <point x="308.1337" y="435.07516" type="curve"/>
      <point x="278.75214" y="458.72375"/>
      <point x="239.70587" y="467.89307"/>
      <point x="202" y="467" type="curve"/>
      <point x="159.17648" y="465.98572"/>
      <point x="116.71459" y="452.65057"/>
      <point x="81" y="429" type="curve"/>
      <point x="81" y="429"/>
      <point x="114" y="368"/>
      <point x="114" y="368" type="curve"/>
      <point x="140.25069" y="385.1524"/>
      <point x="170.07515" y="396.80795"/>
      <point x="201" y="402" type="curve"/>
      <point x="216.00638" y="404.51947"/>
      <point x="231.74168" y="405.47617"/>
      <point x="246.15791" y="400.60675" type="curve"/>
      <point x="253.36604" y="398.17206"/>
      <point x="260.1371" y="394.26904"/>
      <point x="265.48807" y="388.86057" type="curve"/>
      <point x="270.83902" y="383.45206"/>
      <point x="274.7198" y="376.49973"/>
      <point x="276" y="369" type="curve"/>
      <point x="277.28708" y="361.46"/>
      <point x="275.9142" y="353.58243"/>
      <point x="272.70132" y="346.64084" type="curve"/>
      <point x="269.48846" y="339.69928"/>
      <point x="264.49753" y="333.66504"/>
      <point x="258.67157" y="328.70862" type="curve"/>
      <point x="247.01959" y="318.79575"/>
      <point x="232.36713" y="313.2553"/>
      <point x="218" y="308" type="curve"/>
      <point x="191.97757" y="298.4813"/>
      <point x="165.57028" y="289.16183"/>
      <point x="142.2789" y="274.15237" type="curve"/>
      <point x="118.98753" y="259.1429"/>
      <point x="98.727295" y="237.60942"/>
      <point x="91" y="211" type="curve"/>
      <point x="85.821045" y="193.16595"/>
      <point x="86.58067" y="173.74878"/>
      <point x="92.58657" y="156.17595" type="curve"/>
      <point x="98.592476" y="138.60313"/>
      <point x="109.74926" y="122.90761"/>
      <point x="124" y="111" type="curve"/>
      <point x="138.28033" y="99.06766"/>
      <point x="155.53865" y="90.93393"/>
      <point x="173.57802" y="86.36339" type="curve"/>
      <point x="191.61739" y="81.79284"/>
      <point x="210.43518" y="80.71308"/>
    </contour>
  </outline>
</glyph>
JoesCat commented 1 year ago

I noted the 1.5hrs of problematic code conversion, but you started with correctly licensed code and you finished where you are now. No code bled-over from libspiro.

Congratulations Fredrick @ctrlcctrlv and Seth @Subject38 for doing a job well done.