search

NaturalHistoryMuseum / pylibdmtx

Read Data Matrix barcodes from Python 2 and 3.
MIT License
145 stars 56 forks source link

Return four vertices and Rect object for rotated images #85

Closed pcumani closed 4 months ago

pcumani commented 1 year ago

Hi

Thanks for creating pylibdmtx. I am using it to extract datamatrix from photos. Since the matrix in the photo can be rotated, I found myself in the situation described in issue #51. I have just implemented the code from @gauti1311 (https://github.com/NaturalHistoryMuseum/pylibdmtx/issues/51#issuecomment-582447839) and @jepperaskdk (https://github.com/NaturalHistoryMuseum/pylibdmtx/issues/51#issuecomment-1052125416) (a big thanks to both of them).

I have added a boolean parameter to choose between returning a rectangle defined as before (left, top, width, height) but encompassing the whole datamatrix: image or the coordinates of the four vertices (P0, P1, P2, P3): image

jepperaskdk commented 1 year ago

Awesome, good job. Will definitely test it out. :)

jepperaskdk commented 1 year ago

Seems to work as expected - getting the 4 corners is also more useful, as you can easily compute the boundingbox in your client-application, rather than in the library. image

I left a comment about the namedtuple.

@quicklizard99 are you the owner of the repository? Can you check this PR out? Problem is described in https://github.com/NaturalHistoryMuseum/pylibdmtx/issues/51

eukatree commented 1 year ago

I was testing this, and noticed that this commit would change the height value from a positive number to a negative number.

Previously, the height would be a positive value, but using this branch, the height values are negative. For example, the test datamatrix gives the following output using this branch:

Decoded(data=b'Stegosaurus', rect=Rect(left=189, top=92, width=96, height=-95)) Stegosaurus Decoded(data=b'Plesiosaurus', rect=Rect(left=333, top=89, width=96, height=-95)) Plesiosaurus

Should this be changed to match the previous outputs? I made a comment on line 192 for the change that would need to be made to do this if proper.

pcumani commented 1 year ago

Just implemented the changes suggested by @jepperaskdk and @EukaDesigns

GraciousGpal commented 1 year ago

Can we get this merged ?

pcumani commented 1 year ago

Hello! Please let me know if any other modification is needed.

alycejenni commented 4 months ago

Hi everyone! I haven't worked on this repository yet, but since this seems to be a major issue and it's been reviewed by 2 peers already, I'm willing to merge it in. Unfortunately I'm not able to release it to PyPI, so it won't be released on there until we can get hold of the package owner.