Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1041203004:
clamp matrix-translate before converting to pmcolor (Closed)
Descriptionclamp matrix-translate before converting to pmcolor
.. thanks to https://codereview.chromium.org/1032593003/ layout failures
BUG=skia:
Committed: https://skia.googlesource.com/skia/+/005b84ec1c3122885d154d84e29a5deed273d7b6
Patch Set 1 #Patch Set 2 : rebase #Messages
Total messages: 13 (4 generated)
reed@chromium.org changed reviewers: + fmalita@chromium.org, mtklein@google.com
The CQ bit was checked by reed@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1041203004/1
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2015-03-31 01:27 UTC
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for src/effects/SkColorMatrixFilter.cpp:
While running git apply --index -3 -p1;
error: patch failed: src/effects/SkColorMatrixFilter.cpp:272
Falling back to three-way merge...
Applied patch to 'src/effects/SkColorMatrixFilter.cpp' with conflicts.
U src/effects/SkColorMatrixFilter.cpp
Patch: src/effects/SkColorMatrixFilter.cpp
Index: src/effects/SkColorMatrixFilter.cpp
diff --git a/src/effects/SkColorMatrixFilter.cpp
b/src/effects/SkColorMatrixFilter.cpp
index
a406eda56491a42b8ac2507d4014dc1f25e585ab..fc8aba258eb896b631bc2d26fb3bbe4e6feec6a8
100644
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -272,6 +272,10 @@ static Sk4f unpremul(const SkPMFloat& pm) {
return Sk4f(pm) * Sk4f(scale, scale, scale, 1);
}
+static Sk4f clamp_0_255(const Sk4f& value) {
+ return Sk4f::Max(Sk4f::Min(value, Sk4f(255)), Sk4f(0));
+}
+
void SkColorMatrixFilter::filterSpan(const SkPMColor src[], int count,
SkPMColor dst[]) const {
Proc proc = fProc;
if (NULL == proc) {
@@ -294,7 +298,8 @@ void SkColorMatrixFilter::filterSpan(const SkPMColor src[],
int count, SkPMColor
const Sk4f c3 = Sk4f::Load(fTranspose + 12);
const Sk4f c4 = Sk4f::Load(fTranspose + 16); // translates
- SkPMColor matrix_translate_pmcolor = SkPMFloat(premul(c4)).clamped();
+ // todo: we could cache this in the constructor...
+ SkPMColor matrix_translate_pmcolor =
SkPMFloat(premul(clamp_0_255(c4))).clamped();
for (int i = 0; i < count; i++) {
const SkPMColor src_c = src[i];
@@ -317,11 +322,8 @@ void SkColorMatrixFilter::filterSpan(const SkPMColor src[],
int count, SkPMColor
// apply matrix
Sk4f dst4 = c0 * r4 + c1 * g4 + c2 * b4 + c3 * a4 + c4;
- // pin before re-premul (convention for color-matrix???)
- dst4 = Sk4f::Max(Sk4f(0), Sk4f::Min(Sk4f(255), dst4));
-
- // re-premul and write
- dst[i] = SkPMFloat(premul(dst4)).get();
+ // clamp, re-premul, and write
+ dst[i] = SkPMFloat(premul(clamp_0_255(dst4))).get();
}
} else {
const State& state = fState;
On 2015/03/30 19:27:14, I haz the power (commit-bot) wrote:
> Failed to apply patch for src/effects/SkColorMatrixFilter.cpp:
> While running git apply --index -3 -p1;
> error: patch failed: src/effects/SkColorMatrixFilter.cpp:272
> Falling back to three-way merge...
> Applied patch to 'src/effects/SkColorMatrixFilter.cpp' with conflicts.
> U src/effects/SkColorMatrixFilter.cpp
>
> Patch: src/effects/SkColorMatrixFilter.cpp
> Index: src/effects/SkColorMatrixFilter.cpp
> diff --git a/src/effects/SkColorMatrixFilter.cpp
> b/src/effects/SkColorMatrixFilter.cpp
> index
>
a406eda56491a42b8ac2507d4014dc1f25e585ab..fc8aba258eb896b631bc2d26fb3bbe4e6feec6a8
> 100644
> --- a/src/effects/SkColorMatrixFilter.cpp
> +++ b/src/effects/SkColorMatrixFilter.cpp
> @@ -272,6 +272,10 @@ static Sk4f unpremul(const SkPMFloat& pm) {
> return Sk4f(pm) * Sk4f(scale, scale, scale, 1);
> }
>
> +static Sk4f clamp_0_255(const Sk4f& value) {
> + return Sk4f::Max(Sk4f::Min(value, Sk4f(255)), Sk4f(0));
> +}
> +
> void SkColorMatrixFilter::filterSpan(const SkPMColor src[], int count,
> SkPMColor dst[]) const {
> Proc proc = fProc;
> if (NULL == proc) {
> @@ -294,7 +298,8 @@ void SkColorMatrixFilter::filterSpan(const SkPMColor
src[],
> int count, SkPMColor
> const Sk4f c3 = Sk4f::Load(fTranspose + 12);
> const Sk4f c4 = Sk4f::Load(fTranspose + 16); // translates
>
> - SkPMColor matrix_translate_pmcolor = SkPMFloat(premul(c4)).clamped();
> + // todo: we could cache this in the constructor...
> + SkPMColor matrix_translate_pmcolor =
> SkPMFloat(premul(clamp_0_255(c4))).clamped();
>
> for (int i = 0; i < count; i++) {
> const SkPMColor src_c = src[i];
> @@ -317,11 +322,8 @@ void SkColorMatrixFilter::filterSpan(const SkPMColor
src[],
> int count, SkPMColor
> // apply matrix
> Sk4f dst4 = c0 * r4 + c1 * g4 + c2 * b4 + c3 * a4 + c4;
>
> - // pin before re-premul (convention for color-matrix???)
> - dst4 = Sk4f::Max(Sk4f(0), Sk4f::Min(Sk4f(255), dst4));
> -
> - // re-premul and write
> - dst[i] = SkPMFloat(premul(dst4)).get();
> + // clamp, re-premul, and write
> + dst[i] = SkPMFloat(premul(clamp_0_255(dst4))).get();
> }
> } else {
> const State& state = fState;
Sorry, you just landed on my Sk4f -> Sk4s refactor.
no expected rebaselines
The CQ bit was checked by reed@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1041203004/20001
Note for Reviewers: The CQ is waiting for an approval. If you believe that the CL is not ready yet, or if you would like to L-G-T-M with comments then please uncheck the CQ checkbox. Waiting for LGTM from valid reviewer(s) till 2015-03-31 02:19 UTC
lgtm. i'll follow up putting things back as Sk4f.
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://skia.googlesource.com/skia/+/005b84ec1c3122885d154d84e29a5deed273d7b6 |
