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 missing cairo-svg dependency to test-ot-color #1788

Merged
merged 1 commit into from Jun 23, 2019

Conversation

jameshilliard
Copy link
Contributor

Should fix:

test-ot-color.cc:40:10: fatal error: cairo-svg.h: No such file or directory
   40 | #include <cairo-svg.h>
      |          ^~~~~~~~~~~~~

@behdad
Copy link
Member

behdad commented Jun 23, 2019

I don't like configure check for this. @ebraminio can you change to just use cairo PNG backend instead? Or use #ifdefs. That's what hb-view does.

@jameshilliard
Copy link
Contributor Author

cairo PNG would still require a dependency check for this right?

@behdad
Copy link
Member

behdad commented Jun 23, 2019

The cairo PNG backend is always buillt in, so we don't check extra for hb-view and never had any complaints.

These are macros to check for all:

helper-cairo.cc:  #ifdef CAIRO_HAS_PS_SURFACE
helper-cairo.cc:#ifdef CAIRO_HAS_PNG_FUNCTIONS
helper-cairo.cc:  #ifdef CAIRO_HAS_PNG_FUNCTIONS
helper-cairo.cc:  #ifdef CAIRO_HAS_SVG_SURFACE
helper-cairo.cc:  #ifdef CAIRO_HAS_PDF_SURFACE
helper-cairo.cc:  #ifdef CAIRO_HAS_PS_SURFACE

@jameshilliard
Copy link
Contributor Author

The cairo PNG backend is always buillt in

Are you sure? What about when cairo is built with --disable-png?

@behdad
Copy link
Member

behdad commented Jun 23, 2019

You're right. The image backend is always built in, but the PNG functions can be disabled. At any rate, I don't like adding extra checks just for test-ot-colors. The test file can just not save to anything if the necessary cairo features are not available.

@jameshilliard
Copy link
Contributor Author

Updated to use the CAIRO_HAS_SVG_SURFACE macro instead.

@ebraminio
Copy link
Collaborator

ebraminio commented Jun 23, 2019

Hey there, shouldn't cairo header itself be included before checking its definitions?

@jameshilliard
Copy link
Contributor Author

updated, hopefully that works

@ebraminio ebraminio merged commit 8062979 into harfbuzz:master Jun 23, 2019
@ebraminio
Copy link
Collaborator

Merged, thanks for your contribution :)

@jameshilliard jameshilliard deleted the configure-svg branch June 23, 2019 18:47
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.

None yet

3 participants