JuliaImages / DitherPunk.jl

Dithering algorithms in Julia.
MIT License
68 stars 3 forks source link

various performance and memory optimization #10

Closed johnnychen94 closed 3 years ago

johnnychen94 commented 3 years ago

The result visually looks "correct" to me so a double-check would be the best, @adrhill it would be better if you can write some correctness test in the future.

using DitherPunk
using TestImages, ImageTransformations, ImageShow, ImageCore

img = imresize(testimage("cameraman"), (240, 240))

@btime bayer_dithering($img);
# master: 1.490 ms (15 allocations: 969.02 KiB)
# PR: 187.368 μs (15 allocations: 233.62 KiB)

@btime bayer_dithering($img, to_linear=true);
# master: 3.190 ms (15 allocations: 969.02 KiB)
# PR: 279.525 μs (15 allocations: 233.62 KiB)

@btime simple_error_diffusion($img);
# master: 743.018 μs (4 allocations: 232.31 KiB)
# PR: 702.736 μs (5 allocations: 232.41 KiB)

@btime simple_error_diffusion($img, to_linear=true);
# master: 1.753 ms (4 allocations: 232.31 KiB)
# PR: 766.562 μs (5 allocations: 232.41 KiB)

@btime threshold_dithering($img, to_linear=true);
# master: 1.784 ms (7 allocations: 180.39 KiB)
# PR: 155.320 μs (19 allocations: 12.34 KiB)
codecov-commenter commented 3 years ago

Codecov Report

Merging #10 (95bd4ff) into master (72ea1d0) will increase coverage by 2.09%. The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #10      +/-   ##
==========================================
+ Coverage   91.17%   93.26%   +2.09%     
==========================================
  Files           6        7       +1     
  Lines         102      104       +2     
==========================================
+ Hits           93       97       +4     
+ Misses          9        7       -2     
Impacted Files Coverage Δ
src/compat.jl 0.00% <0.00%> (ø)
src/colorspaces.jl 50.00% <50.00%> (+10.00%) :arrow_up:
src/ordered.jl 91.42% <93.33%> (-2.12%) :arrow_down:
src/error_diffusion.jl 100.00% <100.00%> (ø)
src/threshold.jl 91.66% <100.00%> (-0.65%) :arrow_down:

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 72ea1d0...95bd4ff. Read the comment docs.

johnnychen94 commented 3 years ago

The @inbounds annotation in ff21dd7 removes the boundary check during getindex operations (e.g., img[p])

@btime simple_error_diffusion($img);
# master: 743.018 μs (4 allocations: 232.31 KiB)
# before: 702.736 μs (5 allocations: 232.41 KiB)
# after: 481.427 μs (5 allocations: 232.41 KiB)

@btime simple_error_diffusion($img, to_linear=true);
# master: 1.753 ms (4 allocations: 232.31 KiB)
# before: 766.562 μs (5 allocations: 232.41 KiB)
# after: 562.010 μs (5 allocations: 232.41 KiB)
johnnychen94 commented 3 years ago

By the way, I fully agree correctness tests are overdue...

Some packages in JuliaImages use ReferenceTests as a tool to write regression test. For example, ImageBinarization.jl

adrhill commented 3 years ago

Some packages in JuliaImages use ReferenceTests as a tool to write regression test. For example, ImageBinarization.jl

Thanks a lot, that's good to know! I'll open an issue.

adrhill commented 3 years ago

Thank you for this amazing PR!