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

Add several new functions to Core Graphics and Core Text for `font-kit`'s use. #237

Merged
merged 1 commit into from Jul 25, 2018

Conversation

@pcwalton
Copy link
Contributor

pcwalton commented Jul 24, 2018

This bumps the major version of core-text since the previous version of
get_glyphs_for_characters was unsoundly marked safe before and is now marked unsafe.

r? @jrmuizel (or whoever)


This change is Reviewable

@jdm
Copy link
Member

jdm commented Jul 25, 2018

Travis shows some Cargo.toml files that need to be updated with the new package versions, but otherwise this looks fine.

unsafe {
CFArray::wrap_under_get_rule(CTFontCollectionCreateMatchingFontDescriptors(self.0))
let array = CTFontCollectionCreateMatchingFontDescriptors(self.0);
if array.is_null() {

This comment has been minimized.

@jrmuizel

jrmuizel Jul 25, 2018

Collaborator

When can CTFontCollectionCreateMatchingFontDescriptors fail?

@pcwalton pcwalton force-pushed the pcwalton:core-graphics-additions branch from a0ec818 to 8e18273 Jul 25, 2018
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 25, 2018

Review comments addressed.

@pcwalton pcwalton force-pushed the pcwalton:core-graphics-additions branch from 8e18273 to a37aae7 Jul 25, 2018
@@ -16,5 +16,5 @@ crate-type = ["rlib"]
block = "0.1"
bitflags = "1.0"
libc = "0.2"
core-graphics = { path = "../core-graphics", version = "0.16" }
core-graphics = { path = "../core-graphics", version = "0.17" }

This comment has been minimized.

@jdm

jdm Jul 25, 2018

Member

This requires a major version change for cocoa.

…t`'s use.

This bumps the major version of `core-text` since the previous version of
`get_glyphs_for_characters` was unsoundly marked safe before and is now marked
unsafe.
@pcwalton pcwalton force-pushed the pcwalton:core-graphics-additions branch from a37aae7 to 2434720 Jul 25, 2018
@pcwalton
Copy link
Contributor Author

pcwalton commented Jul 25, 2018

I updated the Cocoa version (since it's on 0.x, bumped the minor version).

@jdm
Copy link
Member

jdm commented Jul 25, 2018

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2018

📌 Commit 2434720 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2018

Testing commit 2434720 with merge 1519944...

bors-servo added a commit that referenced this pull request Jul 25, 2018
Add several new functions to Core Graphics and Core Text for `font-kit`'s use.

This bumps the major version of `core-text` since the previous version of
`get_glyphs_for_characters` was unsoundly marked safe before and is now marked unsafe.

r? @jrmuizel (or whoever)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-foundation-rs/237)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2018

☀️ Test successful - status-travis
Approved by: jdm
Pushing 1519944 to master...

@bors-servo bors-servo merged commit 2434720 into servo:master Jul 25, 2018
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
homu Test successful
Details
@@ -271,6 +316,20 @@ impl CTFont {
}
}

pub fn get_vertical_translations_for_glyphs(&self,

This comment has been minimized.

@nox

nox Jul 25, 2018

Member

This method should be unsafe too.

@nox
Copy link
Member

nox commented Jul 25, 2018

Please fix the newly-added unsound method before proceeding with publish of core-text 12.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.