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

Parse histograms with fractional colors #94

Merged
merged 1 commit into from
Jan 29, 2021
Merged

Parse histograms with fractional colors #94

merged 1 commit into from
Jan 29, 2021

Conversation

bunnymatic
Copy link
Contributor

@bunnymatic bunnymatic commented Jan 28, 2021

Problem

For some images with very narrow ranges, it appears ImageMagick can
return histograms that have fractional color entries.

Solution

Make the histogram parser optionally look for integer or float color patterns.

@bunnymatic
Copy link
Contributor Author

bunnymatic commented Jan 28, 2021

I'm working on a test.

The scenario where it came up is taking an image and then reducing colors to 8 and then computing the histogram

Mogrify.open(file)
    |> Mogrify.custom("-background", "white")
    |> Mogrify.custom("-alpha", "remove")
    |> Mogrify.custom("-colors", 8)
    |> Mogrify.custom("+dither")
    |> Mogrify.histogram()

I'll see if I can add something in the test suite to prove this out. I've proven it locally by having my app pull in this branch of Mogrify and my test suite for the app (which was previously failing) runs smoothly.

Problem
--------

For some images with very narrow ranges, it appears ImageMagick can
return histograms that have fractional color entries.

Solution
--------

Make the histogram parser optionally look for integer or float color patterns.
@bunnymatic bunnymatic marked this pull request as ready for review January 28, 2021 03:48
%{"alpha" => 255, "blue" => 129, "count" => 4915, "green" => 129, "hex" => "#7F8181", "red" => 127},
%{"alpha" => 255, "blue" => 150, "count" => 5913, "green" => 148, "hex" => "#939496", "red" => 147},
%{"alpha" => 255, "blue" => 191, "count" => 8142, "green" => 192, "hex" => "#BDC0BF", "red" => 189},
%{"alpha" => 255, "blue" => 244, "count" => 58242, "green" => 245, "hex" => "#F4F5F4", "red" => 244}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: If you take this test and apply to master, you should get the error I encountered

test .histogram with fractional rgb values (MogrifyTest)
     test/mogrify_test.exs:504
     ** (Protocol.UndefinedError) protocol Enumerable not implemented for nil of type Atom. This protocol is implemented for the following type(s): HashSet, Range, Map, Function, List, Stream, Date.Range, HashDict, GenEvent.Stream, MapSet, File.Stream, IO.Stream
     code: |> histogram

@bunnymatic
Copy link
Contributor Author

👍 Tanks!

@route
Copy link
Member

route commented Jan 29, 2021

Looks good! Thank you!

@route route merged commit 507c13b into elixir-mogrify:master Jan 29, 2021
@bunnymatic bunnymatic deleted the bug/extract_histogram_data_fails branch January 30, 2021 04:00
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

2 participants