Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1609573002:
Fix SkAlphaThresholdFilter bounds handling. (Closed)
|
Created:
4 years, 11 months ago by Stephen White Modified:
4 years, 11 months ago Reviewers:
robertphillips CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionFix SkAlphaThresholdFilter bounds handling.
SkAlphaThresholdFilter was always allocating a mask texture
of the same size as the source texture. In addition to
potentially wasting VRAM, this could cause the mask to be
offset from the source texture, if the resulting bounds
were a different size than the source texture.
The fix is to allocate a mask texture only as large as the
bounds, and to offset it to the bounds origin on draw.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1609573002
Committed: https://skia.googlesource.com/skia/+/1ea67a31c527f4d5d77c59a3ea3a12e39308e8c5
Patch Set 1 #Patch Set 2 : Speculative Win32 fix #Patch Set 3 : Add raster fallback for surface creation failure in GM #Patch Set 4 : Clear surface to white to fix serialize-8888 config #
Total comments: 4
Patch Set 5 : Fix nits #Messages
Total messages: 25 (13 generated)
Description was changed from ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture the same size as the source texture. Since it was ignoring clipping, this would cause the mask to be offset from the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it by the bounds origin. BUG=skia: ========== to ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture the same size as the source texture. Since it was ignoring clipping, this would cause the mask to be offset from the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it by the bounds origin. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1609573002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1609573002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1609573002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1609573002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
The CQ bit was checked by senorblanco@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1609573002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1609573002/60001
Description was changed from ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture the same size as the source texture. Since it was ignoring clipping, this would cause the mask to be offset from the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it by the bounds origin. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture the same size as the source texture. In addition to potentially wasting VRAM, this could cause the mask to be offset from the source texture, if the resulting bounds were a different size than the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it to the bounds origin on draw. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
senorblanco@chromium.org changed reviewers: + robertphillips@google.com
Rob: PTAL. Thanks!
Description was changed from ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture the same size as the source texture. In addition to potentially wasting VRAM, this could cause the mask to be offset from the source texture, if the resulting bounds were a different size than the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it to the bounds origin on draw. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture of the same size as the source texture. In addition to potentially wasting VRAM, this could cause the mask to be offset from the source texture, if the resulting bounds were a different size than the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it to the bounds origin on draw. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm + nits https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.cpp File gm/imagealphathreshold.cpp (right): https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.... gm/imagealphathreshold.cpp:18: void draw_rects(SkCanvas* canvas) { rectPaint ? https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.... gm/imagealphathreshold.cpp:37: SkPaint paint; Can this be on one line ?
lgtm + nits
Thanks for your review. https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.cpp File gm/imagealphathreshold.cpp (right): https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.... gm/imagealphathreshold.cpp:18: void draw_rects(SkCanvas* canvas) { On 2016/01/19 16:35:15, robertphillips wrote: > rectPaint ? Done. https://codereview.chromium.org/1609573002/diff/60001/gm/imagealphathreshold.... gm/imagealphathreshold.cpp:37: SkPaint paint; On 2016/01/19 16:35:15, robertphillips wrote: > Can this be on one line ? Done.
The CQ bit was checked by senorblanco@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from robertphillips@google.com Link to the patchset: https://codereview.chromium.org/1609573002/#ps80001 (title: "Fix nits")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1609573002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1609573002/80001
Message was sent while issue was closed.
Description was changed from ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture of the same size as the source texture. In addition to potentially wasting VRAM, this could cause the mask to be offset from the source texture, if the resulting bounds were a different size than the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it to the bounds origin on draw. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkAlphaThresholdFilter bounds handling. SkAlphaThresholdFilter was always allocating a mask texture of the same size as the source texture. In addition to potentially wasting VRAM, this could cause the mask to be offset from the source texture, if the resulting bounds were a different size than the source texture. The fix is to allocate a mask texture only as large as the bounds, and to offset it to the bounds origin on draw. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/1ea67a31c527f4d5d77c59a3ea3a12e39308e8c5 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/1ea67a31c527f4d5d77c59a3ea3a12e39308e8c5 |
