-
Notifications
You must be signed in to change notification settings - Fork 625
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
Extract the FemtoVG renderer into a separate crate #2142
Conversation
dcd32a0
to
0b1d1e2
Compare
This is deliberately still a draft and work in progress. One of the things missing is internal documentation. |
0b1d1e2
to
5f8e9a9
Compare
5f8e9a9
to
212ddee
Compare
(don't forget to add the entry in the publish.sh) |
0a7009a
to
fb20256
Compare
internal/renderers/femtovg/build.rs
Outdated
|
||
fn main() { | ||
// Setup cfg aliases | ||
cfg_aliases! { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an objection to the patch, but while i think cfg_alias is great, having a build.rs just for that might have a significant impact on compile time. I haven't measured it though. So I don't know if it is worth it or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are 7 uses of #[cfg(use_fontconfig)]
. I could replace those with the full expression, although I think that makes the code less readable. I'll post a patch on top and then we can see.
@@ -21,7 +21,7 @@ path = "lib.rs" | |||
[features] | |||
wayland = ["winit/wayland", "glutin/wayland", "copypasta/wayland", "i-slint-renderer-skia?/wayland"] | |||
x11 = ["winit/x11", "glutin/x11", "glutin/glx", "copypasta/x11", "i-slint-renderer-skia?/x11"] | |||
renderer-winit-femtovg = ["femtovg", "fontdb", "libc", "yeslogic-fontconfig-sys", "winapi", "dwrote", "imgref", "unicode-script", "ttf-parser", "rgb", "i-slint-core/box-shadow-cache"] | |||
renderer-winit-femtovg = ["i-slint-renderer-femtovg"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renderer-winit-femtovg = ["i-slint-renderer-femtovg"] | |
renderer-winit-femtovg = ["dep:i-slint-renderer-femtovg"] |
But if we do that, we should make it consistent with other features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel very strong about this, but it sounds like something that should be applied to all crates. That said, I don't mind if we don't use dep: everywhere - at least so far I haven't been confused by the mix :)
This will be needed for a future experiment. Unlike the Skia renderer, which operates on raw window handles, the FemtoVG renderer exposes a different interface where it assumes that the caller takes care of the OpenGL context state. This means more boilerplate remains in the winit backend, including the glutin dependency. The upside is that it will allow using the FemtoVG renderer in environments without glutin. In order to work in an environment without fontconfig or memmap, the crate has two features: - fontconfig (set when we anticipate fontconfig to be available at run-time and libloading being available at compile time). - diskfonts (set when we want to be able to load fonts from disk) The winit crate enables fontconfig on "Linux" and diskfonts on !wasm.
3ff2e61
to
603d619
Compare
This will be needed for a future experiment. Unlike the Skia renderer, which operates on raw window handles, the FemtoVG renderer exposes a different interface where it assumes that the caller takes care of the OpenGL context state. This means more boilerplate remains in the winit backend, including the glutin dependency. The upside is that it will allow using the FemtoVG renderer in environments without glutin.