Skip to content

Conversation

behdad
Copy link
Member

@behdad behdad commented Nov 29, 2017

Uncompiled, untested.

Fixes #628

New API:
hb_coretext_font_create()

@behdad behdad requested a review from drott November 29, 2017 07:13
Copy link
Collaborator

@drott drott left a comment

Choose a reason for hiding this comment

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

Could you put a comment somewhere how this is supposed to be used, i.e. use this API not mixed with hb_coretext_face_create API for example?

Otherwise looks okay to me.

if (unlikely (hb_object_is_inert (font)))
return font;

/* Let there be dragons here... */
Copy link
Collaborator

Choose a reason for hiding this comment

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

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Normally that value (ctfont) is automatically created on-demand by HarfBuzz generic layer. We are sneaking in and setting it here. It works and is correct, but is fragile if the rest of the code changes....

@behdad
Copy link
Member Author

behdad commented Nov 29, 2017

@ebraminio since when we started -Werror'ing? So, Mac build has been broken since, because of the deprecation warnings?

@behdad
Copy link
Member Author

behdad commented Nov 29, 2017

Ok I see I'm the one who added that...

@behdad
Copy link
Member Author

behdad commented Nov 29, 2017

Can we add -Wno-deprecated-declarations to OS X builds?

@behdad
Copy link
Member Author

behdad commented Nov 29, 2017

#345

behdad added a commit that referenced this pull request Nov 29, 2017
@ebraminio
Copy link
Collaborator

Can we add -Wno-deprecated-declarations to OS X builds?

Does having access to a macOS terminal help you perhaps to try that or #486? If so, I guess CircleCI provides macOS machines with SSH access.

@behdad
Copy link
Member Author

behdad commented Dec 1, 2017

Can we add -Wno-deprecated-declarations to OS X builds?

Does having access to a macOS terminal help you perhaps to try that or #486? If so, I guess CircleCI provides macOS machines with SSH access.

Seems like I fixed it. ssh to mac would be nice if it works. But if it's too much work to figure out every time I needed every six months, easier to just sit next to drott physically somewhere in the world and do it together.

@ebraminio
Copy link
Collaborator

Turned out CircleCI's macOS support needs some fee unlike TravisCI :(

@behdad behdad force-pushed the ctfont branch 5 times, most recently from 5bb73e7 to 2efa1ef Compare December 2, 2017 19:01
Fixes #628

New API:
hb_coretext_font_create()
@behdad behdad merged commit d5e2930 into master Dec 2, 2017
@behdad behdad deleted the ctfont branch December 2, 2017 23:04
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.

3 participants