Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1861253003:
Use a faster repeat tiler when translate only matrix. (Closed)
DescriptionUse a faster repeat tiler when translate only matrix.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1861253003
Committed: https://skia.googlesource.com/skia/+/303c979b82368c8cb0fddc56845e47ec44a69d85
Patch Set 1 #Patch Set 2 : Calculate repeat count correctly. #Patch Set 3 : comments #
Total comments: 6
Patch Set 4 : Address comments. #Messages
Total messages: 28 (14 generated)
Description was changed from ========== Use a faster repeat tiler when translate only matrix. BUG=skia: ========== to ========== Use a faster repeat tiler when translate only matrix. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Calculate repeat count correctly.
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1861253003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1861253003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
comments
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1861253003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1861253003/40001
Description was changed from ========== Use a faster repeat tiler when translate only matrix. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Use a faster repeat tiler when translate only matrix. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
herb@google.com changed reviewers: + fmalita@chromium.org, mtklein@google.com
This is the second attempt for the repeat tiler. The previous version did not pass asan. If you look at revision #2, this did pass asan.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... File src/core/SkLinearBitmapPipeline.cpp (right): https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline.cpp:597: { Nit: brace on prev line, per Skia style. https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... File src/core/SkLinearBitmapPipeline_tile.h (right): https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline_tile.h:292: SkASSERT(SkScalarFloorToScalar(span.startX()) == 0.0f); Nit: double-paste? https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline_tile.h:311: span.offset(-fXMax * SkScalar(repeatCount)); Nit: maybe use a const SkScalar advance = fXMax * SkScalar(repeatCount) to avoid repetition.
Address comments.
https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... File src/core/SkLinearBitmapPipeline.cpp (right): https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline.cpp:597: { On 2016/04/07 00:56:45, f(malita) wrote: > Nit: brace on prev line, per Skia style. MikeR suggested I move the brace down so he can see where the params stop and the function starts. https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... File src/core/SkLinearBitmapPipeline_tile.h (right): https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline_tile.h:292: SkASSERT(SkScalarFloorToScalar(span.startX()) == 0.0f); On 2016/04/07 00:56:45, f(malita) wrote: > Nit: double-paste? Done. https://codereview.chromium.org/1861253003/diff/40001/src/core/SkLinearBitmap... src/core/SkLinearBitmapPipeline_tile.h:311: span.offset(-fXMax * SkScalar(repeatCount)); On 2016/04/07 00:56:45, f(malita) wrote: > Nit: maybe use a const SkScalar advance = fXMax * SkScalar(repeatCount) to avoid > repetition. Done.
The CQ bit was checked by herb@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from fmalita@chromium.org Link to the patchset: https://codereview.chromium.org/1861253003/#ps60001 (title: "Address comments.")
The CQ bit was unchecked by herb@google.com
The CQ bit was checked by herb@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1861253003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1861253003/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by herb@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1861253003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1861253003/60001
Message was sent while issue was closed.
Description was changed from ========== Use a faster repeat tiler when translate only matrix. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== Use a faster repeat tiler when translate only matrix. 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/+/303c979b82368c8cb0fddc56845e47ec44a69d85 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://skia.googlesource.com/skia/+/303c979b82368c8cb0fddc56845e47ec44a69d85 |
