Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Gaussian Blur's bug which take half of the blur radius compared to the standard, should match Core Image's behavior #2927

Merged
merged 2 commits into from Jan 16, 2020

Conversation

dreampiggy
Copy link
Contributor

@dreampiggy dreampiggy commented Jan 15, 2020

New Pull Request Checklist

  • I have read and understood the CONTRIBUTING guide

  • I have read the Documentation

  • I have searched for a similar pull request in the project and found none

  • I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)

  • I have added the required tests to prove the fix/feature I am adding

  • I have updated the documentation (if necessary)

  • I have run the tests and they pass

  • I have run the lint and it passes (pod lib lint)

This merge request fixes / refers to the following issues: ...

Pull Request Description

See W3C Standard: https://www.w3.org/TR/SVG11/filters.html#feGaussianBlurElement

The gaussian blur should use 3 times box convolve, which the box size should be equal to the following fomula:

let d = floor(s * 3*sqrt(2*pi)/4 + 0.5)

But however, currrent we calculate with this fomula instead, which is wrong:

let d = floor((s * 3*sqrt(2*pi)/4 + 0.5) / 2)

@dreampiggy
Copy link
Contributor Author

Sample image:

clouds

Blur Radius: 3 (which is current behavior when use sd_blurredImageWithRadius with radius 6, which is wrong)

gaussian_blur

Blur Radius: 6 (which is the actually radius 6)

blurredImage_35

@codecov
Copy link

codecov bot commented Jan 15, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@7e3482d). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2927   +/-   ##
=========================================
  Coverage          ?   84.04%           
=========================================
  Files             ?       65           
  Lines             ?     6404           
  Branches          ?        0           
=========================================
  Hits              ?     5382           
  Misses            ?     1022           
  Partials          ?        0
Flag Coverage Δ
#ios 84% <100%> (?)
#tvos 83.71% <100%> (?)
Impacted Files Coverage Δ
SDWebImage/Core/SDAnimatedImage.m 86.87% <ø> (ø)
SDWebImage/Core/UIImage+Transform.m 80.67% <100%> (ø)

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 7e3482d...b8b7438. Read the comment docs.

@dreampiggy dreampiggy merged commit 9a18e62 into SDWebImage:master Jan 16, 2020
@dreampiggy dreampiggy changed the title Fix gaussian blur radius bug Fix Gaussian Blur's bug which double the blur radius compared to the standard Jan 16, 2020
@dreampiggy dreampiggy changed the title Fix Gaussian Blur's bug which double the blur radius compared to the standard Fix Gaussian Blur's bug which take half of the blur radius compared to the standard, should match Core Image's behavior Jan 16, 2020
@dreampiggy dreampiggy added this to the 5.5.0 milestone Jan 16, 2020
@dreampiggy dreampiggy deleted the fix_GaussianBlur_radius_bug branch January 16, 2020 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants