Skip to content

Update to latest Parley version. #936

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

Closed
wants to merge 1 commit into from
Closed

Conversation

xStrom
Copy link
Member

@xStrom xStrom commented Apr 25, 2025

In order to deal with #933 I think we need some Parley changes. However, we're currently using parley v0.3.0 which is a bit different than what main looks like. So in anticipation of soon needing a Parley update I wanted to split the work and first make a PR that imports all the unrelated changes.

In addition to Parley being updated to the latest main revision, this PR also updates Winit and AccessKit to match what Parley expects.

The breaking AccessKit changes were trivial, just a few extra arguments to pass in.

Parley changes required a bit more work. Especially notable is that vertical metrics are no longer rounded. The vertical metrics change alone caused half of Masonry's tests to fail. However, by doing glyph baseline rounding in Masonry as insisted by Chad I got the failing test count down to 9. Those are caused by line spacing changes, which is expected behavior.

Here's an example of how baseline rounding improves pixel alignment (1x scale factor original output, GIF has been been nearest neighbor upscaled).

change

Copy link
Contributor

@PoignardAzur PoignardAzur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the APIs involved, but these changes all seem minor.

LGTM.

@mwcampbell
Copy link
Contributor

I'm going to need the AccessKit update for my own work, so would like to get this merged quickly. Thanks.

@mwcampbell
Copy link
Contributor

One question, now that I'm looking more closely: Why update accesskit_winit only to 0.24 and not the latest, 0.26?

@xStrom
Copy link
Member Author

xStrom commented Apr 29, 2025

This PR here probably won't be merged. I'm driving Parley in a different direction in parley#344 and if that lands we won't need many of the changes I had to make in this PR here.

However, we can update Parley to the March 25 version, which will still get us the AccessKit update. I opened #941 to do that.

Regarding accesskit_winit, I did the bare minimum in this (and in #941) that was needed for Parley to limit the amount of work and the review burden. Once the update to 0.24 is done we can definitely see about updating to 0.26.

github-merge-queue bot pushed a commit that referenced this pull request Apr 29, 2025
With #936 stalling but Matt wanting AccessKit changes I decided to split
out parts of #936 into this PR. This brings in Parley from March 25th,
2025 and that gets us the AccessKit update.
@theoparis
Copy link

theoparis commented May 1, 2025

This also fixes linebender/parley#340 for me on MacOS. Without this PR, xilem's main branch with masonry and xilem causes objc crashes 👀

@DJMcNab
Copy link
Member

DJMcNab commented May 8, 2025

@xStrom can you update this to main before we land Parley 0.4.0? I want to be sure that there isn't anything we've missed in that release.

I'm about to do the same for Vello

@xStrom
Copy link
Member Author

xStrom commented May 8, 2025

As Parley changed I did it in a new PR - #953.

@DJMcNab
Copy link
Member

DJMcNab commented May 8, 2025

Closing this PR then, as it's been superseded. Obviously feel free to re-open if that's wrong

@DJMcNab DJMcNab closed this May 8, 2025
github-merge-queue bot pushed a commit that referenced this pull request May 8, 2025
This is basically the upcoming Parley 0.4.0, but right now pointing
towards the latest git rev.

The snapshots have changed because line height layout code improved in
Parley.

Fixes #933
Closes #936
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

Successfully merging this pull request may close these issues.

5 participants