Use "blur_sigma" instead of "blur_radius" in Shadow.

[heke123]

This is a bug-fix. (flutter/flutter#81145)
issue: #81145

The Skia 'SkMaskFilter::MakeBlur()' interface expects a 'blur_sigma'
value but currently we pass it a 'radius'.

The skt::TextShadow class also direcly passes its |fBlurRadius|
directly to SkMaskFilter::MakeBlur() without converting it to a
'blur_sigma', which makes the |fBlurRadius| member actually means
"blur_sigma" instead of "blur_radius".

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.
• The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[flar]

This PR is modifying some of the code in the imported third party libraries so I think @jason-simmons should be looking at this and considering the import status of the code in whether these changes will make tracking the third party code harder in the long run.

Also, at least one of the data members still uses the old "radius" naming even though it is holding a "sigma" value. Should it be changed to Sigma as well?

[jason-simmons]

LGTM

The code in flutter/third_party originated from external sources, but it is effectively now a fork and is not actively kept in sync with the upstream.

However, the SkParagraph TextShadow.fBlurRadius field is defined in code that lives in Skia's repository. It looks like SkParagraph copied Libtxt's use of the "blur radius" name and is also passing that value to SkMaskFilter::MakeBlur which is expecting a sigma value.
(https://github.com/google/skia/blob/master/modules/skparagraph/src/TextLine.cpp#L368)

SkParagraph should be updated similarly to this PR. But that will need to happen separately in the Skia sources.

[flar]

Would it make sense to file a Skia bug about that and then a "follow-on" Flutter bug to fix the naming in this code when that gets addressed?

[jason-simmons]

Sent a PR to update SkParagraph: https://skia-review.googlesource.com/c/skia/+/401496

[jason-simmons]

The change to update SkParagraph and the engine to use TextShadow.fBlurSigma instead of fBlurRadius has landed.

This PR will need to be rebased on top of the latest engine.

[flar]

We have an issue here. A 0 radius does not convert into a 0 sigma, so the test for 0 no longer works.

One thing I notice is that the dart conversion function used above says that it is based on the Skia version and provides a link to the Skia conversion code. If you take a look at the Skia equations, they make sure that a radius of 0 produces a sigma of 0 and that a sigma of less than 0.5 produces a radius of 0.

The net effect should be that we might want to update our Dart conversion function to match the 0 => 0 behavior of the Skia function and to change this test to blur_sigma <= 0.5?

[heke123]

make sence, thanks! the test should be "blur_sigma > 0.5" which means it is a valid value. right?

[flar]

Right, > 0.5 means it will do something. I got that backwards.

[heke123]

@flar I didn't figure out why the "Linux Android AOT Engine" failed. Seems I don't have the privilege to re-trigger it, could you please give a help? Thanks.

[flar]

LGTM

[flar]

The test failure can be resolve by rebasing the fix to the top of tree.

[flar]

This will merge when the tree goes green. Thanks!