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

webrender fails to build with --default-features=false + features = "pathfinder" #2617

Closed
fschutt opened this issue Apr 5, 2018 · 6 comments
Closed
Assignees

Comments

@fschutt
Copy link
Contributor

@fschutt fschutt commented Apr 5, 2018

[dependencies.webrender]
git = "https://github.com/servo/webrender"
rev = "f9ddc5261ad2cd35c60851c4091b682c900dbb0a"
features = ["pathfinder"]

Since now both pathfinder and webrender link to freetype, this fails to build with:

error: multiple packages link to native library `freetype`, but a native library can be linked only once

package `freetype-sys v0.6.0`
    ... which is depended on by `pathfinder_font_renderer v0.1.0 (https://github.com/pcwalton/pathfinder#fcfd157c)`
    ... which is depended on by `webrender v0.57.2 (https://github.com/servo/webrender?rev=f9ddc5261ad2cd35c60851c4091b682c900dbb0a#f9ddc526)`
    ... which is depended on by `azul v0.1.0 (file:///home/felix/Development/azul)`
links to native library `freetype`

package `servo-freetype-sys v4.0.3`
    ... which is depended on by `freetype v0.4.0`
    ... which is depended on by `webrender v0.57.2 (https://github.com/servo/webrender?rev=f9ddc5261ad2cd35c60851c4091b682c900dbb0a#f9ddc526)`
    ... which is depended on by `azul v0.1.0 (file:///home/felix/Development/azul)`
also links to native library `freetype`

So, my next course of action was to disable the default freetype feature (also note that with the migration to pathfinder, it shouldn't be a default feature anymore IMO):

[dependencies.webrender]
git = "https://github.com/servo/webrender"
rev = "f9ddc5261ad2cd35c60851c4091b682c900dbb0a"
default-features = false
features = ["pathfinder"]

Which fails to build with a few compile errors:

   Compiling pathfinder_partitioner v0.1.0 (https://github.com/pcwalton/pathfinder#fcfd157c)
error[E0599]: no method named `add_native_font` found for type `&mut pathfinder_font_renderer::FontContext<webrender_api::FontKey>` in the current scope
   --> /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/glyph_rasterizer.rs:867:27
    |
867 |                 drop(self.add_native_font(&font_key, (*native_font_handle).clone().0));
    |                           ^^^^^^^^^^^^^^^

error[E0609]: no field `0` on type `webrender_api::NativeFontHandle`
   --> /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/glyph_rasterizer.rs:867:54
    |
867 |                 drop(self.add_native_font(&font_key, (*native_font_handle).clone().0));
    |                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0599]: no method named `compute_scale` found for type `glyph_rasterizer::FontTransform` in the current scope
   --> /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/platform/unix/font.rs:280:49
    |
280 |         let (x_scale, y_scale) = font.transform.compute_scale().unwrap_or((1.0, 1.0));
    |                                                 ^^^^^^^^^^^^^
    |
   ::: /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/glyph_rasterizer.rs:83:1
    |
83  | pub struct FontTransform {
    | ------------------------ method `compute_scale` not found for this

error[E0599]: no method named `invert_scale` found for type `glyph_rasterizer::FontTransform` in the current scope
   --> /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/platform/unix/font.rs:289:44
    |
289 |             let mut shape = font.transform.invert_scale(x_scale, y_scale);
    |                                            ^^^^^^^^^^^^
    |
   ::: /home/felix/.cargo/git/checkouts/webrender-c3596abe1cf4f320/f9ddc52/webrender/src/glyph_rasterizer.rs:83:1
    |
83  | pub struct FontTransform {
    | ------------------------ method `invert_scale` not found for this

error: aborting due to 4 previous errors

error: Could not compile `webrender`.
warning: build failed, waiting for other jobs to finish...
error: build failed

rustc 1.25 stable
Debian 9.4 (note: in the pathfinder repo, the add_native_font uses macOS Quartz fonts?)

@fschutt
Copy link
Contributor Author

@fschutt fschutt commented Apr 5, 2018

Well, it seems that freetype is still needed on Linux. The pathfinder feature really only works on macOS. So, I'd rather just update the version of freetype in the pathfinder repository.

@fschutt
Copy link
Contributor Author

@fschutt fschutt commented Apr 5, 2018

Even with https://github.com/pcwalton/pathfinder/pull/78 merged, I still can't get rid of the "no method named XXX" errors. But at least I don't get version conflicts anymore.

@pcwalton pcwalton self-assigned this Apr 5, 2018
@pcwalton
Copy link
Collaborator

@pcwalton pcwalton commented Apr 5, 2018

I'll look into this.

@fschutt
Copy link
Contributor Author

@fschutt fschutt commented Apr 8, 2018

I know what the issue is. servo expects a FontContext. When using pathfinder, the FontContext is swapped to be a PathfinderFontContext. However, the PathfinderFontContext is missing the add_native_font function (which can be trivially copied from webrender/src/platform/unix/font.rs to pathfinder). The add_native_font function is missing for non-macOS targets. I've fixed this for Linux here, but for Windows, this is currently just a shim so that it should compile, the add_native_font function does nothing on windows.

@fschutt
Copy link
Contributor Author

@fschutt fschutt commented Apr 8, 2018

https://github.com/pcwalton/pathfinder/blob/fcfd157c702d5ca6cabb30c0239af2d7308f3eb4/font-renderer/src/core_graphics.rs#L71-L79

Next, the signature for add_native_font should look like this:

pub fn add_native_font(&mut self, font_key: &FK, handle: NativeFontHandle) -> Result<(), ()> { }

instead of:

pub fn add_native_font(&mut self, font_key: &FK, handle: CGFont) -> Result<(), ()> { }

... because CgFont is only available on macOS. If you look closely, NativeFontHandle is defined (in webrender) as:

// every other OS, except for windows
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct NativeFontHandle {
    pub pathname: String,
    pub index: u32,
}

// macOS special case
#[cfg(target_os = "macos")]
#[derive(Clone)]
pub struct NativeFontHandle(pub CGFont);

.. which means that NativeFontHandle is just a wrapper around CGFont on macOS. So, instead of accepting a CGFont, it's better to accept a NativeFontHandle to keep the function signature consistent with the other OS.

@pcwalton
Copy link
Collaborator

@pcwalton pcwalton commented Apr 8, 2018

pcwalton added a commit to pcwalton/webrender that referenced this issue Apr 9, 2018
pcwalton added a commit to pcwalton/webrender that referenced this issue Apr 9, 2018
pcwalton added a commit to pcwalton/webrender that referenced this issue Apr 9, 2018
bors-servo added a commit that referenced this issue Apr 10, 2018
Port the Pathfinder branch to Linux (FreeType).

Closes #2617.

r? @glennw

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2636)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.