Octree quantization #7
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #7 +/- ##
===========================================
- Coverage 93.61% 33.83% -59.79%
===========================================
Files 4 6 +2
Lines 47 133 +86
===========================================
+ Hits 44 45 +1
- Misses 3 88 +85
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
src/octree.jl
Outdated
| end | ||
|
|
||
| function (alg::OctreeQuantization)(img::AbstractArray) | ||
| return octreequantisation!(img; numcolors = alg.numcolors) |
There was a problem hiding this comment.
For the other quantisation methods, calling the quantizer directly (or calling quantize(img, alg)) currently returns a colorscheme and not the quantized image.
This is why other methods don't have in-place modifying versions and why ColorQuantization currently doesn't have a quantize! function.
We'll have to brainstorm possible API designs for different use cases:
- A): image to colorscheme (current master)
- B): image to quantized image (this PR?)
- C): image to "fitted" quantizer. This quantizer could then
- take a color and return the closest quantized color
- take an image and quantize it
There was a problem hiding this comment.
I think I would want to do this and reluctant to provide API for third given incase we do return a function that acts as quantizer, that will compare each pixel with all quantized colors for similarity which might make it unusable but for a single color it would work:
img, palette = quantize(img, alg; return_palette=true)
img = quantize(img, alg; return_palette=false)
quantize!(img, alg) # we can return palette here too but looks unusual
src/octree.jl
Outdated
| return octreequantisation!(img; numcolors = alg.numcolors) | ||
| end | ||
|
|
||
| function octreequantisation!(img; numcolors = 256, precheck::Bool = false) |
There was a problem hiding this comment.
precheck should be made part of the OctreeQuantization struct, otherwise it will be inaccessible to users.
Minor nitpick: To keep the spelling consistent with the American English package name, "quantization` should be spelled with a "z".
There was a problem hiding this comment.
- Do we always want to run precheck then? There is a big added cost to precheck's unique call
- Sure thing I'll update the quantisation with quantization
| return OctreeQuantization(OCTREE_DEFAULT_COLORSPACE, numcolors; kwargs...) | ||
| end | ||
|
|
||
| function (alg::OctreeQuantization)(img::AbstractArray) |
There was a problem hiding this comment.
I've tried running
using ColorQuantization, TestImages
img = testimage("fabio")
img = RGB{N0f8}.(img)
alg = OctreeQuantization(64)
alg(img)however, the algorithm doesn't terminate after waiting several minutes.
There was a problem hiding this comment.
The problem is fabio having high number of colors combined with how I prune the tree, lines using allleaves(root) is where trouble is
julia> unique(img)
37499-element Array{RGB{N0f8},1} with eltype RGB{N0f8}:
RGB{N0f8}(0.541,0.506,0.51)
RGB{N0f8}(0.459,0.337,0.325)
RGB{N0f8}(0.51,0.341,0.306)
RGB{N0f8}(0.455,0.341,0.278)
RGB{N0f8}(0.435,0.349,0.306)
⋮
RGB{N0f8}(0.898,0.569,0.424)
RGB{N0f8}(0.906,0.58,0.416)
RGB{N0f8}(0.925,0.596,0.424)
RGB{N0f8}(0.89,0.561,0.388)
There was a problem hiding this comment.
It's actually horrifyingly slow ;-;
| if (eltype(img) != RGB{N0f8}) | ||
| error("Octree Algorithm requires img to be in RGB colorspace") | ||
| end |
There was a problem hiding this comment.
Maybe octreequantization could cast input images to RGB{N0f8} for the user instead of deepcopying them. This way, this error could be avoided automatically.
There was a problem hiding this comment.
We could surely but then we are quantising a different image entirely in a different colorspace, we def could avoid deepcopying if we are not gonna accept it if it not right type
src/octree.jl
Outdated
| r, g, b = map(p -> bitstring(UInt8(p * 255)), channelview([img[in]])) | ||
| rgb = r[1] * g[1] * b[1] |
There was a problem hiding this comment.
Why are strings used here to represent UInt8 colors?
There was a problem hiding this comment.
I don't use it as a way to identify color but as a identifier of case number of which possible cases are ["000","001"...."111"] eight different cases of octree at each level. More details on this can be found here: http://delimitry.blogspot.com/2016/02/octree-color-quantizer-in-python.html
There was a problem hiding this comment.
Albeit I do agree that using rgb = r[1] * g[1] * b[1] and then doing root.children[i].data[1] == rgb is misleading
There was a problem hiding this comment.
But now that I think about it, do we even need the bitstring?.......
This is designed to only work for RGB
Issues to address