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

Bump image to 0.24? #44

Closed
AntoniosBarotsis opened this issue Dec 15, 2022 · 7 comments
Closed

Bump image to 0.24? #44

AntoniosBarotsis opened this issue Dec 15, 2022 · 7 comments

Comments

@AntoniosBarotsis
Copy link

I'm not sure how hard this would be or what changes need to be made (I think I read somewhere that the image crate introduced some breaking changes somewhere in that version?), just making sure there's a reason for this :)

@piderman314
Copy link
Owner

Well it seems to compile just fine, but for some reason one of the tests fails. I'll have to have a look at it.

@AntoniosBarotsis
Copy link
Author

Nice! 0.24 seems to have some minor performance improvements over 0.23 for some stuff I was working on so that's a plus for the crate!

@AntoniosBarotsis
Copy link
Author

AntoniosBarotsis commented Dec 21, 2022

I took a short look at the test, not sure if you already found out about this or if it even helps but it seems like the vector of Candidates is slightly different between the 2 versions.

# 0.23.14
[src\detect\linescan.rs:126] &candidates = [
    QRFinderPosition {
        location: Point {
            x: 182.0,
            y: 161.0,
        },
        module_size: 15.5,
        last_module_size: 0.0,
    },
    QRFinderPosition {
        location: Point {
            x: 386.5,
            y: 157.0,
        },
        module_size: 14.678571428571429,
        last_module_size: 0.0,
    },
    QRFinderPosition {
        location: Point {
            x: 184.5,
            y: 370.0,
        },
        module_size: 14.25,
        last_module_size: 0.0,
    },
]

# 0.24.0
[src\detect\linescan.rs:126] &candidates = [
    QRFinderPosition {
        location: Point {
            x: 182.0,
            y: 161.0,
        },
        module_size: 15.5,
        last_module_size: 0.0,
    },
    QRFinderPosition {
        location: Point {
            x: 386.5,
            y: 155.0,
        },
        module_size: 14.892857142857142,
        last_module_size: 0.0,
    },
    QRFinderPosition {
        location: Point {
            x: 184.5,
            y: 370.0,
        },
        module_size: 14.25,
        last_module_size: 0.0,
    },
]

The second candidate's y position (and the modulo) is slightly different (157 vs 155).

After some dbg printing, it seems that this function generates different results between the 2 versions.

@piderman314
Copy link
Owner

It seems there are some very minute changes in the way the grayscale images are handled within the image create between versions 0.23 and 0.24. In most cases it causes no issue except for this one test that fails which was already borderline unreadable. Still, the algorithm managed to correctly read the QR. To be honest I don't really have time to take a deep dive into the changes in the image crate to figure out where things go wrong.

I'll think about updating Cargo.toml to accept both 0.23 and 0.24 and disabling this one test. You can then use the newer image crate while accepting a slight regression in the ability to read QRs.

@pieterdd
Copy link

pieterdd commented Mar 10, 2023

A screenshotting crate I'm using depends on image 0.24.x, while bardecoder seems to use 0.23.14 in my environment. This seems to have triggered the following error:

struct image::buffer_::ImageBuffer and struct ImageBuffer have similar names, but are actually distinct types?

If the one test with the borderline unreadable QR code is disabled, are there any remaining impediments to a 0.24 upgrade?

@piderman314
Copy link
Owner

Apologies, this issue slipped by mind during the holidays. Version 0.4.1 of bardecoder should now allow usage of version 0.24 of the image crate.

@pieterdd
Copy link

Awesome, thank you! This fixed the error I saw.

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

No branches or pull requests

3 participants