houqp / leptess

Productive and safe Rust binding for leptonica and tesseract
https://houqp.github.io/leptess/leptess/index.html
MIT License
259 stars 29 forks source link

Take u32 instead of i32 in leptonica::Box #13

Closed kangalio closed 4 years ago

kangalio commented 4 years ago

As far as I can tell there is no reason to accept negative coordinates

ccouzens commented 4 years ago

Thank you for the issue 🙂

Do you mean here? https://github.com/houqp/leptess/blob/master/src/leptonica.rs#L61-L63

I think it's i32 because the underlying function uses i32 https://docs.rs/leptonica-sys/0.3.1/leptonica_sys/fn.boxCreateValid.html (l_int32 = c_int = i32)

The underlying function returns none if given negative width or height (https://tpgit.github.io/Leptonica/boxbasic_8c_source.html#l00176).

Box::new returns None if given negative width or height.

I'm not sure why the c library uses signed ints. But because it uses signed ints, it makes sense for leptess to also use signed ints. If leptess used unsigned ints then there would be values that couldn't be passed to the c library (anything between 2147483647 and 4294967295).

Does that help explain why it is? 🙂

kangalio commented 4 years ago

Yeah that explains it. Actually I realized exactly this when I tried to implement this issue in my fork. :)