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

Suggestion: Distribute the raw bindings as a crate and continue to use them as dependency. #6

Closed
pragmatrix opened this issue Feb 3, 2019 · 10 comments

Comments

@pragmatrix
Copy link
Member

I did an experiment and separated the canvas implementation and the hello.rs into a another dependent project and it went along fine, Rust seems to be awesome in this regard, the skia.lib and the skiabinding.lib were all included into the resulting rlib, and the linker flags were propagated.

The improvements I seek are:

  • To separate the the evolution of the raw binding generation (including feature control!) from the future safe bindings or other simplified bindings like the already existing canvas implementation.
  • To simplify IDE support and development. For example, CLion completely freaks out with the included raw bindings inside the same project, it shows errors and the IDE completion does not work, but as soon they are in a dependent create, everything is fine.
  • To reduce build times for creating dependent crates, running build.rs for every change seems to be too heavy.
  • To simplify builds. We could run the binding generation every time (i.e remove the INIT_SKIA switch). Cargo seems to be intelligent enough to build the project only once for dependent projects as long there are no changes affecting it, though I don't know how reliable that is.
  • To avoid waiting for and using azure-pipelines for uncountable hours and heating up our planet even more.

Note that I am a Rust beginner, and some of the points above may be speculative and based on naive assumptions.

Naming suggestions for the resulting crates and projects:

  • skia-raw, for the raw bindings (we just export everything that binding.rs produces).
  • skia-canvas, for the simplified canvas drawing implementation.
  • skia-safe, for the safe bindings which should closely resemble the Skia API.
@Brooooooklyn
Copy link
Member

I approve your suggestion. More over it, I'm thinking about make skia-raw a prebuilt library which include skia static link file which generated in CI. Build skia is painful if someone want to use skia-raw`.

@pragmatrix
Copy link
Member Author

pragmatrix commented Feb 5, 2019

More over it, I'm thinking about make skia-raw a prebuilt library

I would like that but have no idea how to build a crate with prebuilt libraries for multiple platforms. Do we need to split them up, in skia-raw-{linux,windows,macos}, and then refer them from skia-raw with feature flags?

Some progress on my side: I've built something similar to what I would consider a safe Skia API on top of the bindings and ported the canvas drawing example to it. It's a mess, but as soon I do have some time, I will clean it up and try to formulate how I think we can proceed.

@pragmatrix
Copy link
Member Author

I think we can build on that:

https://github.com/pragmatrix/skia-safe

which is inspired by the binding code in skia-sys and contains the canvas example. It can be compiled with my "vulkan" binding branch:

https://github.com/pragmatrix/rust-skia/pull/3/files

@Brooooooklyn
Copy link
Member

Should we create an organization rust-skia for all these projects?

I would like that but have no idea how to build a crate with prebuilt libraries for multiple platforms

Oh it's easy, we can build static link files in CI, and deploy it to github release with names like: rust-skia-prebuilt-{window/mac/linux}-vx.x.x. And in build.rs we could write like this:

let static_link_file_name = format!("skia-{}.{}", platform, ext);
Command::new("wget")
  .args(&[&format!("https://github.com/rust-skia/rust-raw/releases/download/{}/", version, &static_link_file_name)])
  .stdout(Stdio::inherit())
  .stderr(Stdio::inherit())
  .status()
  .expect("download prebuilt static file fail");

Command::new("mv")
  .arg(&static_link_file_name)
  .arg(&format("skia.{}", ext))
  .stdout(Stdio::inherit())
  .stderr(Stdio::inherit())
  .status()
  .unwrap();

@pragmatrix
Copy link
Member Author

Should we create an organization rust-skia for all these projects?

Yes, I had that on my mind, too, but I need to get my PoC working regarding vulkan, before I am willing to contribute more time to completing the bindings. This might take a while.

There is also that: https://doc.rust-lang.org/cargo/reference/build-scripts.html#overriding-build-scripts, not sure if it fits our purpose, but seems reasonable to think about.

@Brooooooklyn
Copy link
Member

There is also that: https://doc.rust-lang.org/cargo/reference/build-scripts.html#overriding-build-scripts

I think this setting is suitable for our situation.

And I have created rust-skia organization and invited you to join it.

BTW. I'm in the Chinese new year vacation. I will pay more times on our project after the vacation end.

@pragmatrix
Copy link
Member Author

... I need to get my PoC working regarding vulkan

done

@pragmatrix
Copy link
Member Author

@Brooooooklyn please make me an "owner" to the rust-skia organization, thanks!

@pragmatrix
Copy link
Member Author

I am working on the first set of bindings in https://github.com/rust-skia/skia-safe/pull/4. This is far from being complete, and needs a lot of polishing, but I think that I can be there in about one or two weeks.

I am now quite confident that we can pull this off based on the C++ bindgen generated bindings, and therefore I think we should produce something that is compilable out of the box soon. To compile the project, rust-skia and skia-safe need to be picked and the cargo dependency paths had to be adjusted to make this work, this is not usable.

For that to proceed, we need to clearly define how the repository and crate layout should look like.

After thinking a while, and because the unsafe bindings and safe bindings seem to co-evolute together (for example, most new APIs require the addition of new extern "C" functions in bindings.cpp), we should probably put them together into one repository:

So my refined suggestion is to create and maintain two repositories only:

  • Repository: "rust-skia"

    • A Cargo workspace referring to the following libs and crates:
      • Lib "skia-raw", just a submodule reference to the respository "skia-raw".
      • Lib "skia-unsafe", bindgen generated bindings + extern "C" functions needed`.
      • Lib "skia-safe", low-level, safe, bindings.
      • Lib "skia-canvas", high level, safe canvas bindings.
  • Repository: "skia-raw"

    • Refers to skia as a submodule (no changes, if possible just a commit hash to google's skia repository).
    • Builds the skia libraries.
    • Supports the distribution of prebuilt skia library binaries.

@pragmatrix
Copy link
Member Author

The first step for splitting up the crates was made in #18, anything else depends on #49 and the "skia-canvas" is incorporated into skia-safe as an example, of which it may grow out of if someone cares enough.

Closing this in favor of #49.

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

No branches or pull requests

2 participants