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

Leptonica 1.83.0 #6

Merged
merged 27 commits into from
Feb 19, 2023
Merged

Leptonica 1.83.0 #6

merged 27 commits into from
Feb 19, 2023

Conversation

ccouzens
Copy link
Owner

Refactor to support leptonica-1.83.0.

Leptonica 1.83.0 removed field access from the public API, which meant we now need to go through Leptonica's methods for more things.

DanBloomberg/leptonica@e8b670d

#5

Additionally, this allowed me to re-evaluate how we abstract Leptonica's memory patterns. I've introduced the 3 memory wrappers RefCounted, RefCountedExclusive and BorrowedFrom. I think this is simpler, and more correct.


There will be additional PRs coming
https://github.com/ccouzens/tesseract-plumbing/compare/leptonica_1.83.0
houqp/leptess@master...leptonica_1.83.0
https://github.com/antimatter15/tesseract-rs (no PR, not affected)

#5

```bash
LD_LIBRARY_PATH="$(pwd)/../../DanBloomberg/leptonica/local/lib" PKG_CONFIG_PATH="$(pwd)/../../DanBloomberg/leptonica/local/lib/pkgconfig" cargo test
```
I don't think that was safe. Someone could have initialized a new
instance and then called the method, which would have been unsafe.
same as what I did with Pix. So that I can use the original name for the
trait.
The c interface feels old fashioned
In the process I had to change how borrowed box wrapper works. It's now
returned as a ref counted value, so it suddenly needs to implement drop.
I'll need to do the same for Box as well. It turns out I need all 3
cases for tesseract (Cloned (implments drop), Borrowed and Normal).
I'll need to do this from tesseract-plumbing
It's not correct as it needs to perform validation. And I don't like
that it could panic
```
LD_LIBRARY_PATH="$(pwd)/../../DanBloomberg/leptonica/local/lib" PKG_CONFIG_PATH="$(pwd)/../../DanBloomberg/leptonica/local/lib/pkgconfig" bash -c 'cargo test --release && valgrind --leak-check=yes --error-exitcode=1 --leak-check=full --show-leak-kinds=all "$(find target/*/deps/ -executable -name 'leptonica_plumbing-*')"'
```
I noticed that imagelib_versions could return NULL, so handle that.

Document testing with valgrind
But in doing so, I found that I wasn't consistent and thought of a
better way.
It's nice not to have the behaviour traits (BorrowedPix(a)) with this
redesign.
Don't replace it, as it's now documented in the code.
Useful for Leptess which gets pix from 2 different methods and wants to
store them the same way.
It's a breaking change, and this has been stable for long enough
@ccouzens ccouzens marked this pull request as ready for review February 17, 2023 15:40
@ccouzens ccouzens requested a review from houqp February 17, 2023 15:40
@ccouzens ccouzens mentioned this pull request Feb 17, 2023
@ccouzens ccouzens merged commit 033498e into main Feb 19, 2023
@ccouzens ccouzens deleted the leptonica_1.83.0 branch February 19, 2023 19:11
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

1 participant