Update to mupdf 1.25 #121
Update to mupdf 1.25 #121
Conversation
|
Looks like the build failure is caused by ArtifexSoftware/mupdf@0e21bb9 |
|
Hmm, yeah this may be quite difficult. I don't have any windows computers, so I'm not able to try out msys2 builds on my local machine. And the fact that everything besides msys2 is working correctly is somewhat concerning - maybe there's some documentation somewhere about building on msys2 that I just need to read? I'm not sure. I'll take another look at this later. |
|
cc @vinxv in case you know how to resolve this. |
After investigating the issue, I've identified two necessary adjustments for successful compilation of MuPDF 1.25.x under MSYS2: Required Changes:
This change replaces the Microsoft-specific integer suffix I anticipate addressing this issue this Saturday. Of course, I would greatly appreciate if someone else implements these fixes beforehand. Thank you. |
It seems we were thinking about the same thing at the same time - I actually just added this in d37faf4, just a few minutes before your comment. I tested that my commit did, in fact, produce the expected features flags (such as I also feel like this should have worked, but maybe these flags are somehow being lost somewhere in the various layers of build scripts and makefiles? I'm not certain.
According to the comment associated with the commit that made this change, this suffix works with mingw, but I think you are right that this wouldn't work on clang on windows. I can file a revert PR for upstream mupdf and see what they think. |
|
Ah, looks like I was confused - the commit I linked actually fixes the issue you mentioned; we just need to pull it in. I'll upgrade this branch to get that commit in. |
|
The MuPDF makefile does not appear to support In /* We assume that pretty much any X86 or X64 machine has SSE these days. */
#ifndef ARCH_HAS_SSE
#if defined(_M_IX86) || defined(_M_AMD64) || defined(_M_X64)
#define ARCH_HAS_SSE 1
#endif
#endif
#ifndef ARCH_HAS_SSE
#define ARCH_HAS_SSE 0
#endifOne approach to customize SSE support is to pass We can compile MuPDF in the MSYS2 environment using: make HAVE_X11=no HAVE_GLUT=no XCFLAGS="-DARCH_HAS_SSE=0"However, using Additionally, during compilation, we may encounter an "argument list too long" issue: /bin/sh: line 1: /ucrt64/bin/ar: Argument list too longThe MuPDF community has already addressed this issue: ArtifexSoftware/mupdf@a0b3ae1 We can resolve this by adding the In summary, here are the specific changes to address these issues: @@ -114,6 +114,7 @@ fn build_libmupdf() {
"HAVE_X11=no".to_owned(),
"HAVE_GLUT=no".to_owned(),
"HAVE_CURL=no".to_owned(),
+ "USE_ARGUMENT_FILE=yes".to_owned(),
"verbose=yes".to_owned(),
];
@@ -176,6 +177,9 @@ fn build_libmupdf() {
make_flags.push(format!("XCFLAGS={}", c_flags.to_string_lossy()));
make_flags.push(format!("XCXXFLAGS={}", cxx_flags.to_string_lossy()));
+ #[cfg(all(target_os="windows", any(target_arch="x86", target_arch="x86_64")))]
+ make_flags.push("XCFLAGS='-msse4.1'".to_string());
+
// Enable parallel compilation
if let Ok(n) = std::thread::available_parallelism() {
make_flags.push(format!("-j{}", n)); |
|
Thanks for this @itsjunetime. I'm now able to compile I usually build all my executables for one app under Linux (podman with Successful build for Mac OS x86_64 with this prBuild log excerpt for Mac OS with this PRFailed build for Mac OS x86_64 with main or 0.4.4I first followed comments from @tangent360 on issue 95, prior attempting the build due to import errors. Build log exceprt |
|
Let's proceed without the msys2 tests (comment them out in github actions) to unblock #131? what do you guys think? |
|
Idk about the others, but the 0.5 release isn't so urgent for me. I'm gonna look into it, maybe there is some way to still make windows compile. |
|
I got it working. Hope it's ok if I push to your branch? |
|
Thanks @ginnyTheCat, do you mind explaining a bit about the fix here so we can use it as a reference in future version updates? |
|
I was just gonna edit my message with an explaination so you caught me hahahaha. To my surprise cc doesn't pass any target features to the c compiler atm. This would allow us to remove The main weirdness debugging this is that mupdf assumes x86 to always come with sse4.1 if not explicitly set (which is a fair enough assumption i guess, but not on mingw apparently). Another confusing thing is that the define mupdf cares about is what a mess :( |
|
I'm getting locally on macOS 15 Apple clang version 17.0.0 (clang-1700.0.13.3). Probably related to llvm/llvm-project#74361, should be fixed by #133 |
|
Yea, there is so many neon feature flags and nowhere it's documented which ones work and do what where. @itsjunetime just commented out https://github.com/messense/mupdf-rs/blob/main/mupdf-sys/build.rs#L51 this line, and I think that's the best option for now then. |
|
Thank y'all so much for getting this through the finish line - I was kinda at a loss for how to actually get it working, so I really appreciate the work you put into it. |
This updates to mupdf 1.25, mainly so that we can use the new
fz_search_stext_page_cbfunction (since the old searching interface was quite clunky). There's also a lot of other changes in this release (see here) but since we haven't added bindings for the other new stuff yet, we won't really get any of these benefits.I feel pretty good about the ffi fuckery I had to do to get this new search interface working, but obviously some second eyes would still be appreciated :)
Also, it seems that mupdf 1.25 does some computations about search result quads slightly differently, resulting in some different results that I adjusted to here. Most of these result in a change of ~0.1, but some resulted in a change of ~7, which is probably not a big issue but just maybe something to be aware of. I'd be happy to chalk it up to some slight differences in text rendering or whatever.