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

cmake: use of imported Cairo target #5352

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Mar 11, 2025

Use imported Cairo target, including:

  • Major update of FindCairo, to include all available Cairo sub-libraries
  • Use CMake's FindFontconfig, drop local find module

@echoix
Copy link
Member

echoix commented Mar 11, 2025

Looks fine in general, I might just want to understand and compare what's different with respect to the new upstream file. https://github.com/WebKit/WebKit/blob/bc82205e7ef89756a44ccf963003c2cb439efe06/Source/cmake/FindCairo.cmake

I can't do it right now on a mobile, but I'll try to get to it. Unless someone is faster than me ;)

Great work otherwise!

@nilason
Copy link
Contributor Author

nilason commented Mar 11, 2025

This FindCAIRO is really custom made for GRASS, (besides variable name case changing) reflecting behaviour of autotools build:

grass/configure.ac

Lines 1765 to 1816 in 5d794e9

# Enable Cairo display driver option
LOC_CHECK_USE(cairo,Cairo,USE_CAIRO)
CAIROINC=
CAIROLIB=
if test -n "$USE_CAIRO"; then
cairo="cairo"
pkgs="cairo-ft cairo-fc cairo-pdf cairo-ps cairo-svg fontconfig"
if test -n "$USE_X11"; then
pkgs="$pkgs cairo-xlib cairo-xlib-xrender"
fi
for pkg in $pkgs ; do
if $PKG_CONFIG --exists $pkg ; then
cairo="$cairo $pkg"
fi
done
# With Cairo includes directory
#FIXME: somehow quote dirs with spaces in $cairo ?
CAIROINC=`$PKG_CONFIG --cflags $cairo`
LOC_CHECK_INC_PATH(cairo,cairo,CAIROINC)
LOC_CHECK_INCLUDES(cairo.h,Cairo,$CAIROINC)
# With Cairo library directory
CAIROLIB=`$PKG_CONFIG --libs $cairo`
LOC_CHECK_LIB_PATH(cairo,cairo,CAIROLIB)
LOC_CHECK_LDFLAGS(cairo,cairo,CAIROLIB)
LOC_CHECK_FUNC(cairo_create,,,,,$CAIROLIB,[:])
LOC_CHECK_FUNC(cairo_xlib_surface_create_with_xrender_format,,,,,$CAIROLIB,
[CAIRO_HAS_XRENDER=1],[CAIRO_HAS_XRENDER=])
AC_SUBST(CAIRO_HAS_XRENDER)
LOC_CHECK_FUNC(cairo_xlib_surface_get_xrender_format,,,,,$CAIROLIB,
[CAIRO_HAS_XRENDER_SURFACE=1],[CAIRO_HAS_XRENDER_SURFACE=])
AC_SUBST(CAIRO_HAS_XRENDER_SURFACE)
CAIROLIB="$CAIROLIB $CAIROLDFLAGS"
fi # $USE_CAIRO
AC_SUBST(CAIROINC)
AC_SUBST(CAIROLIB)
AC_SUBST(USE_CAIRO)
# Done checking Cairo

Adding cairo-ft, cairo-fc, cairo-pdf, cairo-ps, cairo-svg, cairo-xlib and cairo-xlib-xrender libraries when available.

@echoix
Copy link
Member

echoix commented Mar 11, 2025

Right, in that case, I don't see why I would delay this more than that.

echoix
echoix previously approved these changes Mar 11, 2025
@@ -94,7 +94,7 @@ endif()
# Graphics options
option(WITH_X11 "Build with X11 support ." ${x11_default_option_enabled})
option(WITH_OPENGL "Build with opengl support ." ON)
option(WITH_CAIRO "Build with cairo support ." ON)
option(WITH_CAIRO "Build with Cairo support" ON)
Copy link
Member

Choose a reason for hiding this comment

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

We might want to change the lines around to match this format in a later PR, for example the two lines above had the same formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed.

@nilason
Copy link
Contributor Author

nilason commented Mar 11, 2025

Looks fine in general, I might just want to understand and compare what's different with respect to the new upstream file. https://github.com/WebKit/WebKit/blob/bc82205e7ef89756a44ccf963003c2cb439efe06/Source/cmake/FindCairo.cmake

Now, I actually checked out that link you provided. I haven't seen that version before, but a lot of the changes there are in fact very similar to mine.

- Major update of FindCairo, to include all available Cairo sub-libraries
- Use CMake's FindFontconfig, drop local find module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants