closes https://github.com/JuliaIO/ImageMagick.jl/issues/49 #54
Conversation
|
the only test that failed was due to a load error for julia 0.5 on 32-bit windows. all other versions of julia on all other OSes passed. is there a way to trigger a re-test for just this one case?
|
|
I can restart the whole AppVeyor build, but I don't think AppVeyor lets you restart just one platform. |
src/ImageMagick.jl
Outdated
| T = UFixed8 # always use 8-bit for 8-bit and less | ||
| else | ||
| T = ufixedtype[2*((depth+1)>>1)] # always use an even # of bits (see issue 242#issuecomment-68845157) | ||
| T = UFixed{UInt16,((depth+1)>>1)<<1} # always use an even # of bits (see issue 242#issuecomment-68845157) |
There was a problem hiding this comment.
If ImageMagick ever supports images with more than 16 bits (does it now?), getting rid of the dictionary will make this harder to support. Any particular reason for hardcoding it here?
There was a problem hiding this comment.
supporting >16-bit depths would be easier i think by changing the existing else clause to an elseif depth <= 16 and then adding logic for UFixed{UInt32,..} in a subsequent else clause. i figured we'd do that when the time came, but we could add that now if you want. i don't know whether ImageMagick currently supports it or not.
moreover, the dict uses notation that will be deprecated by JuliaMath/FixedPointNumbers.jl#51.
|
this is what i had in mind re. 32-bit support. there's less hard-coding with this approach than having a dict. the problem is that the 32-bit test is failing. the high 16 bits are the same in the saved and loaded versions, but not the low 16 bits. not sure whether it's on the julia side or ImageMagick's fault. i'm not hugely motivated to fix this as all i need is 16-bit support |
|
Works for me. If that test is causing failures, I'd just say comment it out for now. Then hit merge whenever you feel ready. |
|
what version of imagemagick are you using @timholy ? we should document that it works for that version. i've commented out the 32-bit test, added a comment to it explaining it fails for imagemagick 6.9.5, and added a warning to i don't have push access so i'll leave it to someone else to merge. |
|
Thanks! |
|
Did you intend to comment out |
|
whoops nevermind, looks like the function implementation itself has always been commented out here too |
|
Oh, noticed this:
6.8.9 on my laptop |
3 participants
fixes reading of 16-bit tiffs. previously it worked, but stored them as UFixed8 instead of UFixed16.
while at it, i also fixed some deprecations and removed some duplicated exports.
what i don't understand is why
getimagechanneldepthwas used previously, instead ofgetimagedepth. all tests pass locally so the switch appears not to have broken anything.