Initial attempt to add palette support

[lwabeke]

Allows saving of files using palette table
Example usage below:

using PNGFiles, Images

nclasses = 15
myArray = [UInt8.(mod(x+y, nclasses)) for x=1:640, y=1:480 ]
PNGFiles.save("myfile_pal.png", myArray, palette=colortable)

[Drvi]

Thanks for looking into this! I was just working on supporting 1, 2 and 4 bit_depths for grayscale which could be, iiuc, extended to paletted images as well.

[lwabeke]

This was added in response to this question:
https://discourse.julialang.org/t/saving-2d-arrays-as-indexed-color-images/36399/12

[lwabeke]

But probably needs more work to make sure it doesn't break something, like selecting an invalid bit_depth

[codecov]

Codecov Report

Merging #11 into master will increase coverage by 4.26%.
The diff coverage is 97.22%.

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
+ Coverage   66.40%   70.66%   +4.26%     
==========================================
  Files           4        4              
  Lines         256      300      +44     
==========================================
+ Hits          170      212      +42     
- Misses         86       88       +2     

Impacted Files	Coverage Δ
src/PNGFiles.jl	100.00% <ø> (ø)
src/utils.jl	0.00% <ø> (ø)
src/io.jl	93.17% <97.22%> (+1.75%)	⬆️
src/wraphelpers.jl	66.66% <0.00%> (-2.30%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 684c202...fdfa6cd. Read the comment docs.

[Drvi]

@lwabeke, since this feature is really useful and you laid down the groundwork, I took the liberty and finished it. Currently, we could only support 8-bit palettes; 1, 2 and 4-bit depths need a little bit more packing work to happen.

One thing I'm not happy about: Currently, when you could provide a vector of RBA{N0f8} or RGBA{N0f8} and the image is basically a matrix of zero based indices to it. I restrict the eltype of the image to be UInt8 and explicitly mention the zero-based indexing it in the docstring, but I still feel this can get confusing for users.

cc: @ianshmean

[lwabeke]

Thanks, I took a few stabs in the dark and it seemed very promising, but you took it a lot further.

Yes, the zero-based indexing could be confusing since Julia is mostly 1 based. I guess you could try to autodetect it, if min(image) == 0, assume zero-based, if max(image) == length(palette), assume 1-based, then correct offset. But you are left with the potential inconsistent/weird behaviour if extrema of image doesn't go to the palette edges.
You could alternatively try to use a custom data type/types instead of UInt8 (zero-based and one-based indexing types), but that feels like it is going to cause other hassles for users to cast their image data into these new types. Unless you use it as a very thin wrapper (i.e syntactic sugar to help user understanding):

mutable struct ZeroBasedIndexing
          image :: AbstractArray
end

mutable struct OneBasedIndexing
          image :: AbstractArray
end

PNGFiles.save("myfile_pal.png", ZeroBasedIndexing.(myArray), palette=colortable)
PNGFiles.save("myfile_pal.png", OneBasedIndexing.(myArray), palette=colortable)

with the convert function of OneBasedIndexing applying the -1 offset correction, something like

Base.convert(::Type{ZeroBasedIndexing}, oneBased :: OneBasedIndexing) = ZeroBasedIndexing(oneBased.image.-1)

PNGFiles.save(fname, oneBased :: OneBasedIndexing, palette=colortable) = PNGFiles.save(fname, ZeroBasedIndexing(oneBased.image.-1), palette=colortable)

I'm not sure if you need the Base.convert or if there is a way to get Julia to automatically call the convert, without having to provide the extra specialisation to save

[timholy]

I haven't reviewed this, but just a brief comment to consider leveraging IndirectArrays (which is a tiny package) and OffsetArrays for the zero-based indexing.

[Drvi]

Thanks for the ideas, @timholy @lwabeke. I think that using IndirectArrays and manually changing its index is the way to go.

[IanButterworth]

Sounds like a good strategy. @Drvi I think you have a much better handle on this than me. Feel free to merge when you think it's ready. I'd just suggest that we also check impact on package load time, but I can't imagine that would change much

[IanButterworth]

Seems like travis's windows isn't letting go of the test files before the cleanup happens.

Given clean-up isn't critical on CI, perhaps just disable the cleanup on CI

https://github.com/lwabeke/PNGFiles.jl/blob/30daf9fcb88474b914825a1e7720b5022f5605b1/test/runtests.jl#L53

function is_ci()
    get(ENV, "TRAVIS", "") == "true" ||
    get(ENV, "APPVEYOR", "") in ("true", "True") ||
    get(ENV, "CI", "") in ("true", "True")
end

if !is_ci()
    # Cleanup
    isdir(PNG_TEST_PATH) && rm(PNG_TEST_PATH, recursive = true)
end

[Drvi]

Just fyi, I'm working on this. For symmetry, I'd like to allow reading paletted PNGs as IndirectArrays with OffsetArray palette, but there are some cases in PNGSuite images that give me a hard time.

[Drvi]

Not sure what is going on with the FreeBSD, but I'm gonna merge this so we can move forward. Thanks everybody!

[IanButterworth]

Awesome. Just an afterthought, but is this worth a read/write example in the readme?