-
Notifications
You must be signed in to change notification settings - Fork 54
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
Expose FPDF_GetPageSizeByIndexF and cache DynamicPdfiumBindings #141
Conversation
I don't know why the tests fail, they run fine locally. |
Thank you so much @DorianRudolph , this is superb work. Please accept my apologies for the delay in replying, I have some deadlines on some other projects that are distracting me from pdfium-render. I will reply more fully later, but I really like the idea of caching the dynamic function pointers to improve performance - thank you for doing this. If you're interested in figuring out why the tests are failing in github, you can reproduce the github action workflow locally (at least on a linux machine) by following the instructions at the top of https://github.com/ajrcarey/pdfium-render/blob/master/.github/workflows/build_test.yml. |
No worries. The tests seem to be an unrelated issue: #143 |
Thanks again for preparing this change, @DorianRudolph . I hope to get it merged over the weekend. Question: for the PDFIUM_DYNAMIC_LIB_PATH, does the value of the variable actually matter? I noticed in the build.rs script that you don't seem to actually pass the value of the variable to rustc. |
The path is given here: } else if let Ok(path) = std::env::var("PDFIUM_DYNAMIC_LIB_PATH") {
// Instruct cargo to dynamically link the given library during the build.
println!("cargo:rustc-link-lib=dylib=pdfium");
println!("cargo:rustc-link-search=native={}", path); // <--
} |
Merged changes. Renamed |
First, sorry for combining effectively two PRs into one, but exposing FPDF_GetPageSizeByIndexF also requires changing the bindings.
FPDF_GetPageSizeByIndexF is much faster than loading each page and then obtaining its dimensions. For a large book with hundreds of pages, FPDF_GetPageSizeByIndexF only takes a fraction of a second vs several seconds.
I also noticed that the DynamicPdfiumBindings reload the symbol from libloading for every call. It is much more efficient to store the function pointers in the struct. This should bring the overhead in line with "normal" dynamic linking. Taking the function pointers out of the symbol is "unsafe" but it is fine as long as their lifetime does not exceed that of the Library (see https://users.rust-lang.org/t/multiple-objects-with-interdependent-lifetimes-in-the-same-struct/16507/3).
I created the symbol table with the following script: https://gist.github.com/DorianRudolph/1be46b909764faa5559673936342bc3f
Lastly, you can also use the StaticPdfiumBindings with dynamic linking, for which I added the PDFIUM_DYNAMIC_LIB_PATH environment variable to the build.rs. This does not require any changes besides linker flags.