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

Update image to 0.24 #491

Merged
merged 6 commits into from
Mar 1, 2022
Merged

Conversation

okaneco
Copy link
Contributor

@okaneco okaneco commented Mar 1, 2022

I've broken out the commits to hopefully make it easier for review.

Closes #485


3f2f097, 9fb0ea3

image 0.24 removes the Bgr and Bgra pixel types image-rs/image#1482

'static was largely removed from traits and impl bounds so it's removed here image-rs/image#1597

e501161

Main bulk of changes for migration work

Some trait bounds needed to be added to src/integral_image.rs and src/map.rs functions since a sealed trait Enlargeable was added to the Rgb/Rgba definitions image-rs/image#1651

The following seems to satisfy the requirements.

    Rgba<T>: Pixel<Subpixel = T>,
    T: Primitive,

2a3c5c0

GenericImage::get_pixel_mut is deprecated in 0.24 so I've replaced the two occurrences of it.

61f4863

Replaced regression truth image files.

4 regression tests failed likely due to changes made in Luma calculation to minimize floating point errors image-rs/image#1214.
The failure error output can be read below in the contracted view. The differences are fairly small.

27 of the files changed are truth files from regression tests being regenerated. In test_match_histograms, the center pixel at (177, 170) doesn't match.

Failed regression tests
failures:

---- test_gaussian_blur_stdev_3 stdout ----
thread 'test_gaussian_blur_stdev_3' panicked at 'pixels do not match.

location: (13, 6), actual: 209, expected: 208
location: (13, 7), actual: 210, expected: 209
location: (11, 10), actual: 212, expected: 211
location: (13, 10), actual: 212, expected: 211
location: (0, 11), actual: 213, expected: 212 ', tests\regression.rs:123:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- test_match_histograms stdout ----
thread 'test_match_histograms' panicked at 'pixels do not match.
Actual:
        175  176  177  178  179
     +--------------------------
     |
  168|  102  150  150  156  165
     |
  169|   29   54   67   85  110
     |
  170|   31   58   75  106  126
     |
  171|   28   40   54   85   97
     |
  172|   47   50   62   75   75
     |
Expected:
        175  176  177  178  179
     +--------------------------
     |
  168|  102  150  150  156  165
     |
  169|   29   54   67   85  110
     |
  170|   31   58   71  106  126
     |
  171|   28   40   54   85   97
     |
  172|   47   50   62   75   75
     |
', tests\regression.rs:123:9

---- test_gaussian_blur_stdev_10 stdout ----
thread 'test_gaussian_blur_stdev_10' panicked at 'pixels do not match.

location: (29, 1), actual: 98, expected: 97
location: (23, 2), actual: 135, expected: 134
location: (10, 3), actual: 190, expected: 189
location: (0, 5), actual: 201, expected: 200
location: (5, 5), actual: 198, expected: 197 ', tests\regression.rs:123:9

---- test_sobel_gradients stdout ----
thread 'test_sobel_gradients' panicked at 'pixels do not match.

location: (78, 135), actual: 31, expected: 30
location: (79, 135), actual: 31, expected: 29
location: (80, 135), actual: 57, expected: 55
location: (78, 136), actual: 69, expected: 67
location: (80, 136), actual: 41, expected: 39 ', tests\regression.rs:123:9


failures:
    test_gaussian_blur_stdev_10
    test_gaussian_blur_stdev_3
    test_match_histograms
    test_sobel_gradients

test result: FAILED. 23 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out

cf3bec1

I noticed that I added the bound but it wasn't needed due to Enlargeable not being added to the Luma color define macro definitions in image.

@theotherphil
Copy link
Contributor

Fantastic, thanks! Is this ready for review and merge now?

@okaneco
Copy link
Contributor Author

okaneco commented Mar 1, 2022

Yes, it is 👍

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.

Update to image 0.24
2 participants