servo / pathfinder

A fast, practical GPU rasterizer for fonts and vector graphics
Apache License 2.0
3.61k stars 204 forks source link

Apple M1 MacBook Pro: winit bump #483

Open lancelet opened 2 years ago

lancelet commented 2 years ago

On an Apple M1 MacBook Pro, the winit dependency build fails with:

   Compiling winit v0.19.3
error[E0308]: mismatched types
   --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/view.rs:209:9
    |
205 | extern fn has_marked_text(this: &Object, _sel: Sel) -> BOOL {
    |                                                        ---- expected `bool` because of return type
...
209 |         (marked_text.length() > 0) as i8
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `bool`, found `i8`

error[E0308]: mismatched types
   --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:102:26
    |
102 |             is_zoomed != 0
    |                          ^ expected `bool`, found integer

error[E0308]: mismatched types
   --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:174:57
    |
174 |                 self.window.setFrame_display_(new_rect, 0);
    |                                                         ^ expected `bool`, found integer

error[E0308]: mismatched types
    --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:1290:48
     |
1290 |         window.setFrame_display_(current_rect, 0)
     |                                                ^ expected `bool`, found integer

error[E0308]: mismatched types
    --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:1297:48
     |
1297 |         window.setFrame_display_(current_rect, 0)
     |                                                ^ expected `bool`, found integer

error[E0308]: mismatched types
    --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:1314:48
     |
1314 |         window.setFrame_display_(current_rect, 0)
     |                                                ^ expected `bool`, found integer

error[E0308]: mismatched types
    --> /Users/jsm/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/macos/window.rs:1321:48
     |
1321 |         window.setFrame_display_(current_rect, 0)
     |                                                ^ expected `bool`, found integer

This is fixed in winit v0.24.x (see https://github.com/rust-windowing/winit/issues/1782 where this was reported originally).

So, I bumped winit and made a bunch of changes to catch up with its more recent API. After that, I was able to run some of the examples on an M1 Mac (eg. canvas_minimal is working). See this diff for the changes required to canvas_minimal/src/main.rs: https://github.com/lancelet/pathfinder/commit/720785e5abee0a27759b9ebfbcfe3b4ecd3bff40#diff-53ffc830c4d0e3773cb67c3cedd3e24330ba22eccc7ccb66f731b9586fdf6584

However, there's a circular dependency between both pdf and pathfinder, which is making it difficult for me to complete the refactoring within pathfinder itself. pdf and pdf_render bring in a dependency on winit, and use a part of the pathfinder API that has been made private:

error[E0599]: no method named `points` found for reference `&Contour` in the current scope
  --> /Users/jsm/.cargo/git/checkouts/pdf_render-56211f49c16f7190/a57d54f/render/src/graphicsstate.rs:83:50
   |
83 |                 let clip_polygon = outer_contour.points();
   |                                                  ^^^^^^ private field, not a method

To make things worse, pdf and pdf_render seem to be using patches on top of pathfinder, so that confuses the matter still further.

I don't know what the best way forward is. If pdf depends on pathfinder, perhaps it's best not to use pdf examples in pathfinder itself, but rather put appropriate examples in the pdf package instead? (Or in a third party repository which coordinates a minimum bound on the two?) Any suggestions? If I raised a PR to remove pdf and pdf_render as dependencies for all of pathfinder (ie. removing those examples), would that be accepted?

jdm commented 2 years ago

Pathfinder is not actively maintained, but pdf_render relying on a fork of pathfinder does make it harder. It looks like the API issue is due to the pathfinder fork exposing a method that this repository does not: https://github.com/s3bk/pathfinder/commit/32497ee2d23eae8f3be46cdd4fadf562018ed524 .

What if you made a PR to update winit in pdf_render first? Would that be enough to allow updating the pdf_render dependency in this repository?

lancelet commented 2 years ago

I'll give that a go.

s3bk commented 2 years ago

Yes the points method is a bit of a hack to get nested clip paths working. But I also can't really remove it without breaking clip paths in pdf_render.

s3bk commented 2 years ago

@jdm is there a chance of merging #474 ? If not I have to figure out a way to keep clip paths working without it. Keeping the fork is quite annoying for everyone.

jdm commented 2 years ago

Yep, sorry about that!

ghost commented 2 years ago

This issue isn't fixed, despite merging #474, right? I'm still getting 'mismatched types' errors for winit when trying to run the demo/native example.

s3bk commented 2 years ago

Sorry, I will test it now.

s3bk commented 2 years ago

pdf_render does not have a fixed version of winit, so I would expect that a cargo update should fix it?

s3bk commented 2 years ago

It runs on linux.. until it crashes.

ghost commented 2 years ago

Just running cargo update (either in demo/native or in root) throws the same error for me:

    Updating crates.io index
    Updating git repository `https://github.com/pdf-rs/pdf`
    Updating git repository `https://github.com/servo/surfman`
error: no matching package named `pdf_render` found
location searched: https://github.com/pdf-rs/pdf
required by package `pathfinder_demo v0.1.0 (/Users/leontepe/src/pathfinder/demo/common)`

Edit: running macOS, btw

s3bk commented 2 years ago

well. I am a git "master". Turns out it helps to test in the correct repository ...

I am trying now with https://github.com/servo/pathfinder/pull/490

s3bk commented 2 years ago

It compiles in https://github.com/servo/pathfinder/pull/491 .. but instantly crashes.

The application panicked (crashed).
Message:  attempted to leave type `platform::platform::x11::util::input::PointerState` uninitialized, which is invalid
Location: /home/sebk/.cargo/registry/src/github.com-1ecc6299db9ec823/winit-0.19.3/src/platform/linux/x11/util/input.rs:94
ghost commented 2 years ago

Still getting the winit compilation errors. I mean, the version still seems to be on <0.19.4, right?

ghost commented 2 years ago

Maybe your x11 errors are unrelated?

s3bk commented 2 years ago

Maybe. I will try updating winit to 0.26

s3bk commented 2 years ago

Surfman needs updating too ....

ghost commented 2 years ago

Yeah. Just found the surfman dep as well.

ghost commented 2 years ago

We are using this commit it seems like: https://github.com/servo/surfman/blob/f3df871ac8c3926fe9106d86a3e51e20aa50d3cc/surfman/Cargo.toml

ghost commented 2 years ago

Seems to be the @pcwalton's last commit on the project. That explains it.

ghost commented 2 years ago

Updated to the latest surfman and got past the initial winit errors. Now I'm updating demo/native/src/main.rs to use new winit changes.

s3bk commented 2 years ago

shall I send you my changes so far? I guess not really worth it... and it isn't finished.

ghost commented 2 years ago

You could. I guess it's just a lot of winit changes to accommodate.

ghost commented 2 years ago

Not the winit expert myself. Don't really know which of these errors just need their names changed and which need complete overhaul. Can't tell how much winit has changed since then.

s3bk commented 2 years ago

Same. But it is quite late here and I need to fix my font crate. Maybe it is useful. https://gist.github.com/s3bk/b4749790561b80778275cc4fde5873b8

ghost commented 2 years ago

BTW @s3bk, why are u trying to debug this? Are you interested in this crate because of glyph dilation as well? Or different reason?

s3bk commented 2 years ago

I was just trying to get the demo back working for you.

I use pathfinder via https://github.com/s3bk/pathfinder_view which is working.

The font crate is used to render pdfs... and extract text from it.

austintheriot commented 10 months ago

Hello! Now that the servo project is being reactivated, are bugs like this one expected to be addressed moving forward?

jdm commented 10 months ago

No. Pathfinder is not used in Servo at this point in time,and I'm not aware of plans to integrate it.

austintheriot commented 10 months ago

Thank you. What does Servo currently use for vector graphics rasterization/rendering?

jdm commented 10 months ago

Servo does not perform vector graphic rasterization at this time.

austintheriot commented 10 months ago

@jdm Got it, thank you. How is Servo currently rendering text and vector-based graphics, such as svg? Is that CPU-based currently?

jdm commented 10 months ago

Servo does not support svg at this point. It uses font-kit for rendering text.