Please consider using `static const`'s for SPIRO_CORNER, SPIRO_G4, SPIRO_G2, etc.

[ctrlcctrlv]

Let me begin by saying, this is not really your problem, but rather bindgen's problem. (bindgen does the C interop for Rust, one of the main reasons I think Rust is worth using; without bindgen giving C ABI access it's not that great.)

But, I think it would be good to consider changing it anyway. 🥺😉

Having these as symbols in the library is better than just #define's and makes it easier for bindings to "see" them and work with them. There is also the issue that bindgen really cannot know if 'v' is a signed char or an unsigned char; it is ambiguous. In C this is not a problem. In Rust, it is a headache to convert i8 (c_char) to u8 ('v', etc.).

If you agree @JoesCat, I'm happy to PR this.

Patch: https://github.com/mfeq/libspiro/commit/bb20451f90e7e7b8ea035ced4e65290ac8130732

[jtanx]

You can't use variables like those proposed in switch statements fwiw.

[ctrlcctrlv]

Oh, hmm, that's quite a problem when it comes to general applicability of this patch outside bindgen.

The problem with just having a const though, no static, is that the linker complains about multiple definitions.

Perhaps there's no way to make all software happy in this case.

[JoesCat]

Well, libspiro was created in 2007, and I've tried to maintain backwards compatibility as much as possible - successfully so far (I think). Adding the patch is sort of like having the tail wag the dog, because it's going to break compatibility at a source code as well as at an executable level. If libspiro is to be reusable, we should try to maintain a stable APIs that is usable by FontForge, GIMP, your rust editor, etc, etc, etc...

The best solution to keep everyone happy, is to maintain API stability. Basically, treat libspiro as a black-box, with API definitions written in stone.

It may be more effort to adapt the rust font editor, but as a result, I really think it is the best direction to go. Rethink the problem. There is a solution.

[JoesCat]

it is a headache to convert i8 (c_char) to u8 ('v', etc.).

Possible solution since these are basically the same: [=91 ]=93 a=97 c=99 h=104 o=111 v=118 z=122 {=123 }=125

[JoesCat]

I've grabbed some of this patch (Bezier vs bezier), otherwise most of this is going to break a lot of existing code so I'd have to discard everything else. At this point - Think it's safe to close this issue.

[ctrlcctrlv]

No problem at all. My patch makes bindgen happy, which doesn't matter for you the upstream repo. If it can't be done in a way that's going to satisfy other implementations, it shouldn't be done, and I should just maintain a fork.

[ctrlcctrlv]

(By the way, I think this should be fixed in bindgen and will open a bug there if one doesn't exist.)

[JoesCat]

If not a bug - it seems like is a limitation on what you can do.

[ctrlcctrlv]

Well I think most people are just using bindgen as a base and then editing the file it produces, while I'm trying to make bindgen produce perfect code with no edits afterwards, and distributing exactly the file as it comes out of bindgen. So perhaps it's a "little of column A, little of column B." :wink:

[ctrlcctrlv]

By the way @JoesCat , I've got a much better API which I'm putting in the spiro-rs crate, it allows you to construct a Spiro like:

use spiro::{Point as SpiroPoint, PointType as SpiroPointType, Spiro};
use spiro::bezier::Bezier;
let points = vec![SpiroPoint { x: 0., y: 0., ty: SpiroPointType::G4, ..Default::default() }, 
                  SpiroPoint { x: 100., y: 100., ty: SpiroPointType::G4, ..Default::default() }];
let s = Spiro::<f32>::from_points(pts); // cf. Spiro::from_points_closed
// Rust will automatically call the impl From<Spiro<F>> for Bezier<F>, which internally calls TaggedSpiroCPsToBezier2
let b: Bezier = s.into();

use spiro::bezier::AsSVG as _; // bring trait method into scope
println!("{}", b.as_svg());

[JoesCat]

I started to put together a patch/wedge based off of Raph's 2007 code (relicensed Apache), but decided I need to get other stuff done, however the thought would to to substitute libspiro *.h files with wrapper, something like this:

#define _BEZCTX_INTF_H 1 /* substitute my void version */
void bezctx_moveto(void *bc, double x, double y, int is_open);
#define _BEZCTX_H 1 /* substitute my void version */
struct _bezctx {
    /* Called by spiro to start a contour */
    void (*moveto)(void *bc, double x, double y, int is_open);

...basically - rethinking how to use the same library...

...however, if you have another solution that works....sounds fine.

FYI - if you want others to respect your license, then you should respect other licenses too - base your code off of Raph's relicensed spiro to avoid mixing licenses, or keep the library linked and not merged, otherwise you still need to include the gpl3+ license.

[ctrlcctrlv]

Do you think I've done something wrong here? I wrote quite clearly:

Note: LICENSE file refers only to my build.rs. Refer to libspiro for C code licensing and authorship.

So, only my build script is Apache2. My fork of libspiro is GPL3.

[ctrlcctrlv]

@JoesCat Do you mind helping me find the 2007 Apache 2 code? Not finding it

[ctrlcctrlv]

Nevermind, found it. It's not from 2007 exactly, the re-licensing occurred last year against code from 2007.

https://github.com/raphlinus/spiro

If you're upset with how I worded things, I'll see about using this

[JoesCat]

Hi Fredrick, As you mentioned - you are willing to fork. When I looked at https://github.com/mfeq/spiro-sys/blob/master/src/lib.rs and https://github.com/mfeq/spiro-sys/blob/master/tests/spiro_to_beziers.rs I realized that it won't stop here, and other parts will bleed into spiro. This was clearly GPL+ libspiro code and none of this is mentioned in the license file - don't take it personally - I'd say the same for anyone else.

@JoesCat Do you mind helping me find the 2007 Apache 2 code? Not finding it

If you're upset with how I worded things, I'll see about using this

It's the weekend, I needed to go out, do other things, not hover over a keyboard ;-)

Raph relicensed his code, and as your code progresses I assume there will be code blending, mixing-n-matching, therefore, nobody is going to argue about what belongs to what license if your source is traced back to that code. Otherwise, you'd be seeing similar problems to this - which still needs resolving https://github.com/fontforge/fontforge/issues/4077

[skef]

Just for the record I don't think that https://github.com/fontforge/fontforge/issues/4077 was really left unresolved. The primary blocking issue are the remaining files that started out GPL licensed, which are mostly related to the word list stuff. That is, it's a todo issue and not a legal question.

As it happens no one as yet has felt strongly enough about changing back to BSD to work through the replacing but those resources could still be found.

If that's right the FontForge project is approaching the question with some care. If the language about the per-file licenses hadn't been included the change wouldn't be tractable, but it was so it is. And of course any code copied or derived from Raph's work should take as much care, so that (for example--I haven't looked into this and am not making a claim) GPL-licensed code is not "laundered" through a Rust translator into a different license.

[ctrlcctrlv]

Let's slow down and look at what really happened here.

There's no laundering going on. Actually, the first version was not transpiled, it was bound to the C libspiro which was compiled with a C compiler. It's a binding. I mentioned in mfeq/spiro-sys README.md that the C code is not under the same license. The only part of that code that is Apache2 is build.rs, which does the building. Only that file can be used Apache2, theoretically if someone wants a reference for using bindgen.

The second version, which I'll be releasing soon, is pure Rust and is based on raphlinus/spiro, which is already Apache 2. So, what I'm doing is not "laundering". I'm simply not using the fontforge version and going all the way back to @raphlinus' version because (a) it's simpler and works better with C2Rust and (b) its license fits the goals of my project better.

Hope this clarifies things.

[JoesCat]

Well - glad to see this went from panic mode -> to a better solution that resolved several issues.