Skip to content

Remove X11 and non-Cairo drawing backends#5612

Merged
iorsh merged 9 commits intofontforge:masterfrom
valadaptive:no-x11
Sep 19, 2025
Merged

Remove X11 and non-Cairo drawing backends#5612
iorsh merged 9 commits intofontforge:masterfrom
valadaptive:no-x11

Conversation

@valadaptive
Copy link
Copy Markdown
Contributor

@valadaptive valadaptive commented Sep 5, 2025

Resolves #5611 by just removing support for the X11 backend entirely.

If the eventual goal is to implement everything in GTK, we can remove some X11-exclusive code and get rid of quite a bit of cruft. In my testing, the X11 backend seems a bit broken--it doesn't actually exit the process when you close the windows. The non-Cairo drawing backend renders handles below outlines, and randomly decides to zoom into an arbitrary point:

Peek.2025-09-05.02-39.mp4

This PR is split into a few different parts. I can split them into separate PRs, but as a new contributor, I don't want to spam this repo with a bunch of them at once:

  1. Use unifdef to remove the _NO_LIBCAIRO ifdefs. This flag was already never being set, and not much substantive code was gated behind it.
  2. Unifdef FONTFORGE_CAN_USE_GDK. This removes all the gxdraw code, which was gated behind #ifndef FONTFORGE_CAN_USE_GDK.
  3. Remove the ENABLE_X11 CMake option, which now does nothing.
  4. Remove the UseCairoDrawing preference, and corresponding command-line flag. These already have no effect if ENABLE_X11 is off (which it is by default). Also remove the gxdraw source files (now just stubs) entirely.
  5. Remove GDrawHasCairo, which now always returns true, and all code paths in the charview used for non-Cairo rendering. This gets rid of a lot of cruft. This should be reviewed carefully.

Type of change

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 5, 2025

I didn't review this thoroughly, but the idea looks reasonable. GDK backend has been enabled for at least 6 years, and I guess raw X11 is not really needed anymore.

Still, I'd like to seek additional opinions for such a bold move.

@frank-trampe
Copy link
Copy Markdown
Contributor

@valadaptive, this is something that @iorsh and I have discussed doing in the future and shall likely happen, but it is going to require a little consultation with downstream parties and then a deprecation notice in order to limit disruption. Our limited market power means that changes that break somebody's packaging script can lead to the version getting pinned for a long time.

@valadaptive
Copy link
Copy Markdown
Contributor Author

Which downstream packagers still enable the X11 backend?

@valadaptive
Copy link
Copy Markdown
Contributor Author

Also, as @iorsh mentioned in #5611:

This wouldn't be a real fix though, as the new GTK window comes up and even works, but behaves rather erratically.

The latest master is already broken if the X11 backend is enabled. I definitely understand if you don't want this change being made by a new contributor though.

@skef
Copy link
Copy Markdown
Contributor

skef commented Sep 5, 2025

@frank-trampe Is the implication that #5546 either needs to be backed out or needs an X11-specific work-around?

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 5, 2025

@valadaptive, I was unable to reproduce the arbitrary zoom issue you encountered, but I guess it's reasonably common if you stumbled upon it.

@skef, a build fix for #5611 is possible. It would slowly degrade the user experience with X11 backend (with newer GTK windows working, but buggy) instead of shutting it off at once.

@skef
Copy link
Copy Markdown
Contributor

skef commented Sep 5, 2025

I remain skeptical of the inferences being made about the user base. What significant contact have we even had with the user base over the past, say, 4 years? How much script-level development has shifted to fontTools over the past, say, 7 years? What users who are relying on the X11 back-end specifically wouldn't be adequately served by simply sticking with an older version of the program?

@valadaptive
Copy link
Copy Markdown
Contributor Author

What significant contact have we even had with the user base over the past, say, 4 years?

Perhaps relevant, I read this article on Hacker News, and it seems to show how people see FontForge nowadays.

@frank-trampe
Copy link
Copy Markdown
Contributor

I have no current specific information from downstream users/packagers, just memories of past large changes not going over very well. Having been a victim of a few abrupt removals of non-deprecated features in other products, I am sensitive to the general importance of giving notice of large changes.

FontForge has always been clunky, even when it had fewer features, and it was once a lot buggier than it is now. (Try a version from 2012.) Accordingly, evidence-based disdain for FontForge is nothing new. There are nonetheless a lot of people who still use FontForge, and it is important to avoid unnecessarily inconveniencing them.

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 6, 2025

I did some checks, and of all the major distros I could recall none seems to override the default option of ENABLE_X11=OFF

@frank-trampe
Copy link
Copy Markdown
Contributor

Good. Our informal deprecation has been fruitful.

Copy link
Copy Markdown
Contributor

@iorsh iorsh left a comment

Choose a reason for hiding this comment

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

Excellent job, kudos! Just a few minor comments.

Comment thread cmake/FontForgeConfigure.cmake Outdated
Comment thread fontforgeexe/charview.c
Comment thread fontforgeexe/startui.c Outdated
Comment thread fontforgeexe/startui.c Outdated
Comment thread po/ca.po
I meant to only remove cv->filled.
@valadaptive
Copy link
Copy Markdown
Contributor Author

I've made the requested changes.

I've also noticed that cv->back_img_out_of_date and cv->backimgs seem to be completely unused by the Cairo version of the charview. I'll remove those in a follow-up PR; this one is big enough as-is.

Copy link
Copy Markdown
Contributor

@iorsh iorsh left a comment

Choose a reason for hiding this comment

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

Please also remove a few more defines I missed earlier:

  • _NO_XKB
  • _NO_XINPUT
  • BUILT_WITH_XORG
  • _NO_LIBCAIRO (there are few remnants in startui.c)

The file cmake/backports/3.15.7/FindX11.cmake should probably also go.

Please don't force-push, it break the possibility to do incremental review. Thanks!

@valadaptive
Copy link
Copy Markdown
Contributor Author

I've removed those additional defines (seems like unifdef missed those _NO_LIBCAIROs for some reason) and FindX11.cmake.

I think the entire CMake backports folder can probably go away in another PR--even Debian oldoldstable ships cmake 3.18, and CMake prints a big ugly deprecation warning if we set cmake_minimum_required to anything less than 3.10. Would another PR for that be accepted? Not sure which CMake version to target.

Copy link
Copy Markdown
Contributor

@iorsh iorsh left a comment

Choose a reason for hiding this comment

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

This PR looks good to me. There is MacOS CI failure, but it should fix itself in a few days.

I'll merge this when the CI tests are clear, unless there are extraordinary objections.

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 9, 2025

I've removed those additional defines (seems like unifdef missed those _NO_LIBCAIROs for some reason) and FindX11.cmake.

I think the entire CMake backports folder can probably go away in another PR--even Debian oldoldstable ships cmake 3.18, and CMake prints a big ugly deprecation warning if we set cmake_minimum_required to anything less than 3.10. Would another PR for that be accepted? Not sure which CMake version to target.

CMake was introduced by @jtanx, and I don't know what were his considerations when adding local Find*.cmake files to the build system. My guess is that they were too divergent or just not available on all platforms. I recently tried to remove FindGDK3.cmake, but it didn't go smoothly, so I gave up.

@jtanx, would it be safe to remove these files and rely on the standard CMake distribution?

In any case, I'd prefer to defer such PR until after the official release. We've got enough risks for this release already.

@valadaptive
Copy link
Copy Markdown
Contributor Author

I recently tried to remove FindGDK3.cmake, but it didn't go smoothly, so I gave up.

FindGDK3 is not part of the backports folder. Everything in here should be provided by newer versions of CMake--I tried removing the entire folder on my end and it builds just fine (Fedora 42).

@frank-trampe
Copy link
Copy Markdown
Contributor

Let's also leave any libfontforge changes for a future pull request.

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 10, 2025

Let's also leave any libfontforge changes for a future pull request.

As far as I could understand, this was the issue with #5610. Do we have libfontforge headers here too? I see that the library defines two sets of headers: FONTFORGE_NOINST_HEADERS and FONTFORGE_INST_HEADERS, but they end up in the same list without any differentiation.

@frank-trampe, do you have specific changes in mind?

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 10, 2025

Also, it looks like @jtanx explicitly killed libfontforge in #3855:

The only major exclusion compared to autotools is it does not install headers and pkgconfig details. While it is certainly possible to mimic the old behaviour, I'm not seeing much reason to make libfontforge something that can be developed against. As seen in #3823, FontForge isn't well designed to be used in external projects, and we provide no stability guarantees over what is effectively internal interfaces. I doubt that we even export all required headers at the moment.

It was noticed by downstream maintainers - e.g. https://bugs.archlinux.org/task/65877.html, but some of them probably chose to keep maintaining it internally, hence the versioning libfontforge2, libfontforge3, libfontforge4 etc.

Ubuntu, for example, slowly pushes out the 2019 version - https://launchpad.net/ubuntu/focal/amd64/libfontforge-dev

@frank-trampe
Copy link
Copy Markdown
Contributor

I somehow missed that at the time. It is certainly a problem that we need to fix at some point.

Comment thread fontforgeexe/charview.c

/* Sigh. I have to do my own clipping because at large magnifications */
/* things can easily exceed 16 bits */
static int CVSplineOutside(CharView *cv, Spline *spline) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@valadaptive, are you satisfied that clipping happens efficiently in the Cairo pathway?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Clipping has never happened in the Cairo pathway (on our side at least; Cairo probably does it internally). I haven't changed that in this PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand that. My question is whether efficient rendering requires clipping geometry before passing it to Cairo and, if so, whether we do that. I have done very little work on the rendering pipeline, so I do not know this off-hand.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would assume it doesn't, considering we've never done so. I haven't delved too deeply into 2D vector graphics rendering either, but my understanding is that clipping is moreso a correctness thing than a performance one (e.g. if a shape is displayed clipping the left edge of the screen, you need to know to start the scanline with a fill). Cairo definitely handles that for us.

I may not fully understand the question--if you understand that the Cairo backend has never done clipping on the FontForge side, then why would it start being a performance issue now? Is it just something you noticed and are curious about?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes. Just trying to maximize the value of whatever knowledge you have acquired working on this. It works fine, so there is no reason to dig into this further.

@valadaptive
Copy link
Copy Markdown
Contributor Author

valadaptive commented Sep 11, 2025

Ubuntu, for example, slowly pushes out the 2019 version - https://launchpad.net/ubuntu/focal/amd64/libfontforge-dev

Ubuntu 20.04 will always ship the 2019 version though. That won't change no matter what we do here because any Debian-based distro release will (AFAIK) ship the same major versions of packages for its entire lifecycle, backporting fixes from newer versions if enterprise customers pay them to do so. In 22.04 onwards, libfontforge-dev has been completely dropped. So it's not as if distros are foregoing newer versions of FontForge in order to maintain support for libfontforge--Ubuntu 20.04 still has libfontforge because it ships old versions of all software as a matter of policy.

@frank-trampe
Copy link
Copy Markdown
Contributor

@iorsh, I was originally proposing to leave out the struct linelist changes for a future pull request, but it seems pretty clear that no outside product would use it and that we can make those changes without a separate discussion.

@frank-trampe
Copy link
Copy Markdown
Contributor

Any volunteers to check for valgrind regressions?

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 12, 2025

Any volunteers to check for valgrind regressions?

I valground some simple actions, like opening font, opening char view, copy-paste. The logs appear similar, I couldn't see any new issues.

@frank-trampe
Copy link
Copy Markdown
Contributor

I sent the following message to the FontForge users mailing list last Saturday. There are no responses so far. Are there any other checks or adjustments that we need to make before merging?

Hello, all.

FontForge has supported GDK for some time, and there are considerable difficulties in maintaining the legacy X11 interface. We are considering removing it and mandating GDK and Cairo. FontForge can still run in X11 through GDK even without the direct X11 interface. Downstream packagers and distributors may need to adjust scripts and other packaging. Discussion is currently on pull request #5612 on GitHub. We would normally offer a lengthy deprecation window, and we will do so if there is a compelling reason, but a lot of the work is already done, and we would like to move quickly if there are no deal-breakers.

Best wishes,

Frank

@iorsh
Copy link
Copy Markdown
Contributor

iorsh commented Sep 13, 2025

I'm clear with the merge. There is an unrelated CI failure on master (https://github.com/fontforge/fontforge/actions/runs/17700511538) due to recent OS upgrade to Microsoft Windows Server 2025, but I don't want it to delay this PR any further.

@frank-trampe
Copy link
Copy Markdown
Contributor

Okay. It sounds like everybody is satisfied. Let's squash and merge.

@iorsh iorsh merged commit f56dda2 into fontforge:master Sep 19, 2025
12 of 13 checks passed
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.

Building with ENABLE_X11 no longer works We ought to consider removing the Xorg backend GDK and X11 size difference on X11

4 participants