Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1588633002:
Fix SkXfermodeImageFilter GPU fast path for differing sizes. (Closed)
|
Created:
4 years, 11 months ago by Stephen White Modified:
4 years, 11 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionFix SkXfermodeImageFilter GPU fast path for differing sizes.
The GPU fast path was not doing the correct thing for input bitmaps of
differing sizes. This change brings the fast path in line with the
slow path: use the union of foreground and background bounds as bounds,
offset the draw context by the bounds translation, and translate the
foreground and background independently by their respective offsets.
Finally, we add a texture domain for the background fragment
processor, since we may access texels outside its domain.
Note: this adds two new test cases to the xfermodeimagefilter GM, so
those will need to be rebaselined.
BUG=568196
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1588633002
Committed: https://skia.googlesource.com/skia/+/06d54ad09a03edd62d597b2ba67ad3c05944fd92
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix formatting #
Messages
Total messages: 18 (11 generated)
Description was changed from ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=skia: ========== to ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background FP, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. 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@chromium.org
Description was changed from ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background FP, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background FP, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=568196 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Rob: PTAL. Thanks!
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/1588633002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1588633002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background FP, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=568196 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background fragment processor, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=568196 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
robertphillips@google.com changed reviewers: + robertphillips@google.com
lgtm. Yeah - that method surprised me when I was gpu-ifying it. https://codereview.chromium.org/1588633002/diff/1/gm/xfermodeimagefilter.cpp File gm/xfermodeimagefilter.cpp (right): https://codereview.chromium.org/1588633002/diff/1/gm/xfermodeimagefilter.cpp#... gm/xfermodeimagefilter.cpp:174: SkImageFilter::CropRect cropRect(SkRect::MakeXYWH(10, 10, 60, 60)); \n ?
https://codereview.chromium.org/1588633002/diff/1/gm/xfermodeimagefilter.cpp File gm/xfermodeimagefilter.cpp (right): https://codereview.chromium.org/1588633002/diff/1/gm/xfermodeimagefilter.cpp#... gm/xfermodeimagefilter.cpp:174: SkImageFilter::CropRect cropRect(SkRect::MakeXYWH(10, 10, 60, 60)); On 2016/01/13 20:13:15, robertphillips wrote: > \n ? 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/1588633002/#ps20001 (title: "Fix formatting")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1588633002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1588633002/20001
Message was sent while issue was closed.
Description was changed from ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background fragment processor, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=568196 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Fix SkXfermodeImageFilter GPU fast path for differing sizes. The GPU fast path was not doing the correct thing for input bitmaps of differing sizes. This change brings the fast path in line with the slow path: use the union of foreground and background bounds as bounds, offset the draw context by the bounds translation, and translate the foreground and background independently by their respective offsets. Finally, we add a texture domain for the background fragment processor, since we may access texels outside its domain. Note: this adds two new test cases to the xfermodeimagefilter GM, so those will need to be rebaselined. BUG=568196 GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/06d54ad09a03edd62d597b2ba67ad3c05944fd92 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/06d54ad09a03edd62d597b2ba67ad3c05944fd92 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
