Skip to content

Conversation

@Sergio0694
Copy link
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Follow up from #837 - with some minor updates:
• I've added the Complex64 type, which uses the float for the real and imaginary components, to save memory and processing time.
• Following advice from @antonfirsov, I've added a caching system in the BokehBlurProcessor class, so that the kernel parameters and values can be cached and reused across multiple instances using the same initialization parameters.

I need some guidance on how to proceed with the actual convolution pass (I mean, on how to properly structure it following your repo guidelines and coding style), and on eventual optimization steps I need to be aware of while writing this down (especially with the various pixel operations). Unlike with the other blur effects, both convolution passes here produce a Complex64 result, so some changes are needed in both the custom convolution 2 pass processor we'll need, as well as the actual 1D convolution code to be added to the helper class.

@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #840 into master will decrease coverage by 0.23%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #840      +/-   ##
==========================================
- Coverage   88.87%   88.64%   -0.24%     
==========================================
  Files        1015     1018       +3     
  Lines       44240    44357     +117     
  Branches     3202     3208       +6     
==========================================
  Hits        39319    39319              
- Misses       4200     4317     +117     
  Partials      721      721
Impacted Files Coverage Δ
src/ImageSharp/Primitives/Rational.cs 85.29% <ø> (ø) ⬆️
src/ImageSharp/Primitives/Complex64.cs 0% <0%> (ø)
src/ImageSharp/Processing/BokehBlurExtensions.cs 0% <0%> (ø)
...ssing/Processors/Convolution/BokehBlurProcessor.cs 0% <0%> (ø)

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 8f3658d...c150df9. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Feb 20, 2019

Codecov Report

Merging #840 into master will decrease coverage by 0.23%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #840      +/-   ##
==========================================
- Coverage   88.87%   88.64%   -0.24%     
==========================================
  Files        1015     1018       +3     
  Lines       44240    44357     +117     
  Branches     3202     3208       +6     
==========================================
  Hits        39319    39319              
- Misses       4200     4317     +117     
  Partials      721      721
Impacted Files Coverage Δ
src/ImageSharp/Primitives/Rational.cs 85.29% <ø> (ø) ⬆️
src/ImageSharp/Primitives/Complex64.cs 0% <0%> (ø)
src/ImageSharp/Processing/BokehBlurExtensions.cs 0% <0%> (ø)
...ssing/Processors/Convolution/BokehBlurProcessor.cs 0% <0%> (ø)

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 8f3658d...c150df9. Read the comment docs.

@Sergio0694 Sergio0694 mentioned this pull request Feb 21, 2019
4 tasks
@Sergio0694
Copy link
Member Author

Continues in #842 with a work in progress implementation.

@Sergio0694 Sergio0694 closed this Feb 21, 2019
@Sergio0694 Sergio0694 deleted the feature_bokeh-blur branch February 22, 2019 17:10
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.

1 participant