Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2255803003:
Moving SkBlurImageFilter into core (Closed)
|
Created:
4 years, 4 months ago by vjiaoblack Modified:
4 years, 4 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionMoving SkBlurImageFilter into core
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003
Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48
Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0
Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75
Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e
Committed: https://skia.googlesource.com/skia/+/e1e5c74086ceb7143bf2f7968a324e8c06d33914
Patch Set 1 #Patch Set 2 : also added gpublurutils to core #Patch Set 3 : made req changes #Patch Set 4 : made req changes #Patch Set 5 : rebased off of master #Patch Set 6 : removed src/core/SkBlurImageFilter.h #Patch Set 7 : modified some hidden code #
Total comments: 2
Patch Set 8 : Fixed function call name change #
Total comments: 1
Patch Set 9 : made change #
Total comments: 2
Patch Set 10 : ... made req changes onto correct branch #
Messages
Total messages: 61 (29 generated)
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 ==========
vjiaoblack@google.com changed reviewers: + reed@google.com
reed@google.com changed reviewers: + fmalita@chromium.org
1. move SkBlurIMageFilter.h into src/core
2. add this to SkImageFilter.h
static sk_sp<SkImageFilter> MakeBlur(SkScalar sigmaX, SkScalar sigmaY,
sk_sp<SkImageFilter> input, const CropRect* cropRect = nullptr);
and then change theh impl in SkBlurMaskFilter (i.e. don't need a factory on
SkBlurImageFIlter if we have it on SkImageFilter)
3. I expect we'll need a dummy include/effects/SkBlurImageFIlter.h to stay for a
while, for clients that explicitly give that path... then we need a bug to
remind us to update the callers (at least in blink/chrome) so we can eventually
remove their #include
looks pretty good. I think SkBlurImageFilter.h needs to stay in include/effects for compatibility with chrome's hard-coded paths. I do like that it should just contain a call to SkImageFilter::MakeBlur(...)
*resending*
*reupload of most current version*
lgtm
The CQ bit was checked by vjiaoblack@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/2263283002/ by mtklein@google.com. The reason for reverting is: It looks like this breaks our roll into Google3: https://test.corp.google.com/ui#id=OCL:130943857:BASE:130944046:1471881622765....
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 ==========
rs lgtm
The CQ bit was checked by vjiaoblack@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com Link to the patchset: https://codereview.chromium.org/2255803003/#ps120001 (title: "modified some hidden code")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0
Message was sent while issue was closed.
https://codereview.chromium.org/2255803003/diff/120001/src/core/SkBlurImageFi... File src/core/SkBlurImageFilter.cpp (right): https://codereview.chromium.org/2255803003/diff/120001/src/core/SkBlurImageFi... src/core/SkBlurImageFilter.cpp:36: return SkImageInfo::Make(sigmaX, sigmaY, sk_ref_sp<SkImageFilter>(input), cropRect).release(); I would have guessed you wanted SkImageFilter::MakeBlur
Message was sent while issue was closed.
https://codereview.chromium.org/2255803003/diff/120001/src/core/SkBlurImageFi... File src/core/SkBlurImageFilter.cpp (right): https://codereview.chromium.org/2255803003/diff/120001/src/core/SkBlurImageFi... src/core/SkBlurImageFilter.cpp:36: return SkImageInfo::Make(sigmaX, sigmaY, sk_ref_sp<SkImageFilter>(input), cropRect).release(); On 2016/08/22 18:19:57, reed1 wrote: > I would have guessed you wanted > > SkImageFilter::MakeBlur Done.
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 ==========
The CQ bit was checked by vjiaoblack@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org, reed@google.com Link to the patchset: https://codereview.chromium.org/2255803003/#ps140001 (title: "Made function name change")
The CQ bit was unchecked by vjiaoblack@google.com
The CQ bit was checked by vjiaoblack@google.com
The CQ bit was unchecked by vjiaoblack@google.com
The CQ bit was checked by vjiaoblack@google.com
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/2266063002/ by vjiaoblack@google.com. The reason for reverting is: Fixed it wrong, needs to revert to re-discuss and re-land..
The CQ bit was checked by vjiaoblack@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75
Message was sent while issue was closed.
https://codereview.chromium.org/2255803003/diff/140001/src/core/SkBlurImageFi... File src/core/SkBlurImageFilter.cpp (right): https://codereview.chromium.org/2255803003/diff/140001/src/core/SkBlurImageFi... src/core/SkBlurImageFilter.cpp:36: return SkImageInfo::MakeBlur(sigmaX, sigmaY, sk_ref_sp<SkImageFilter>(input), SkImageInfo -> SkImageFilter
Message was sent while issue was closed.
A revert of this CL (patchset #8 id:140001) has been created in https://codereview.chromium.org/2265263002/ by vjiaoblack@google.com. The reason for reverting is: Misnamed function..
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 ==========
The CQ bit was checked by vjiaoblack@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org, reed@google.com Link to the patchset: https://codereview.chromium.org/2255803003/#ps160001 (title: "made change")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001) as https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e
Message was sent while issue was closed.
bungeman@google.com changed reviewers: + bungeman@google.com
Message was sent while issue was closed.
https://codereview.chromium.org/2255803003/diff/160001/include/effects/SkBlur... File include/effects/SkBlurImageFilter.h (left): https://codereview.chromium.org/2255803003/diff/160001/include/effects/SkBlur... include/effects/SkBlurImageFilter.h:25: #ifdef SK_SUPPORT_LEGACY_IMAGEFILTER_PTR Still need this.
Message was sent while issue was closed.
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2274603003/ by bungeman@google.com. The reason for reverting is: Breaking internal roll. Still needs 'Create'..
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e ==========
https://codereview.chromium.org/2255803003/diff/160001/include/effects/SkBlur... File include/effects/SkBlurImageFilter.h (left): https://codereview.chromium.org/2255803003/diff/160001/include/effects/SkBlur... include/effects/SkBlurImageFilter.h:25: #ifdef SK_SUPPORT_LEGACY_IMAGEFILTER_PTR On 2016/08/23 17:02:47, bungeman-skia wrote: > Still need this. Done.
The CQ bit was checked by vjiaoblack@google.com
The CQ bit was unchecked by vjiaoblack@google.com
The CQ bit was checked by vjiaoblack@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org, reed@google.com Link to the patchset: https://codereview.chromium.org/2255803003/#ps180001 (title: "... made req changes onto correct branch")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e ========== to ========== Moving SkBlurImageFilter into core BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2255803003 Committed: https://skia.googlesource.com/skia/+/dd3259eb95c3b47e11eefa3b176365a112a32b48 Committed: https://skia.googlesource.com/skia/+/e426babe7552b1cb4e27cdf4e90826feabb9e3b0 Committed: https://skia.googlesource.com/skia/+/4d760175686df8f61a11b66946eb307d89dd2c75 Committed: https://skia.googlesource.com/skia/+/a97a1ab5719d5c355f7900b7f17dec1e467cf57e Committed: https://skia.googlesource.com/skia/+/e1e5c74086ceb7143bf2f7968a324e8c06d33914 ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as https://skia.googlesource.com/skia/+/e1e5c74086ceb7143bf2f7968a324e8c06d33914 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
