Revert "refresh image shader cs/at logic"
This reverts commit 4311f19158
.
Reason for revert: layout test failures seemingly not having to do with bicubic+clamping+sRGB
Original change's description:
> refresh image shader cs/at logic
>
> Was working on SkVM versions of these when I noticed the existing
> SkRasterPipeline code wasn't terribly tidy. The main gist here is that
> we can let SkColorSpaceXformSteps::apply() handle almost all the
> final transformation on the way out of the shader.
>
> I remain a little puzzled why I got a few significant diffs when I tried
> leaving the A8 color unpremul, letting the steps handle matching the
> alpha types instead of me manually with SkRasterPipeline::premul. For
> now I've left that as-is.
>
> Similarly I think we could transform that A8 color ahead of time rather
> than doing that over and over at runtime. This is something I've left
> as a TODO, largely because I don't care enough about coloring A8 to
> investigate right now.
>
> I've inlined all the logic of explaining src-is-normalized into this
> shader (which is its only user). That makes it clearer that we can
> always set that bit when bicubic sampling, since we're clamping anyway.
> This causes some invisible diffs when switching to the optimized sRGB
> transfer function stages, which we may or may not be able to get away
> with without guarding...
>
> Change-Id: Ie6670c6ca5c69958f41aac88324341a10eb3bee1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261763
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Mike Klein <mtklein@google.com>
TBR=mtklein@google.com,brianosman@google.com,reed@google.com
Change-Id: I9c414cb751d9e51219b18dc3d4f54c92d06664ce
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261815
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
parent
02eeac78f6
commit
0ec2c7a2dc
@ -43,6 +43,10 @@ struct SkColorSpaceXformSteps {
|
||||
void apply(float rgba[4]) const;
|
||||
void apply(SkRasterPipeline*, bool src_is_normalized) const;
|
||||
|
||||
void apply(SkRasterPipeline* p, SkColorType srcCT) const {
|
||||
return this->apply(p, SkColorTypeIsNormalized(srcCT));
|
||||
}
|
||||
|
||||
Flags flags;
|
||||
|
||||
bool srcTF_is_sRGB,
|
||||
|
@ -479,37 +479,36 @@ bool SkImageShader::doStages(const SkStageRec& rec, SkImageStageUpdater* updater
|
||||
};
|
||||
|
||||
auto append_misc = [&] {
|
||||
SkColorSpace* cs = info.colorSpace();
|
||||
SkAlphaType at = info.alphaType();
|
||||
|
||||
// Color for A8 images comes from the paint. TODO: all alpha formats? maybe none?
|
||||
// TODO: if ref.fDstCS isn't null, we'll premul here then immediately unpremul
|
||||
// to do the color space transformation. Might be possible to streamline.
|
||||
if (info.colorType() == kAlpha_8_SkColorType) {
|
||||
// The color for A8 images comes from the (sRGB) paint color.
|
||||
p->append_set_rgb(alloc, rec.fPaint.getColor4f());
|
||||
p->append(SkRasterPipeline::premul);
|
||||
cs = sk_srgb_singleton(); // TODO: transform to dstCS here instead of at runtime?
|
||||
at = kPremul_SkAlphaType;
|
||||
} else if (info.alphaType() == kUnpremul_SkAlphaType) {
|
||||
// Convert unpremul images to premul before we carry on with the rest of the pipeline.
|
||||
p->append(SkRasterPipeline::premul);
|
||||
}
|
||||
|
||||
// This is an unessential optimization... it's logically safe to set this to false.
|
||||
// But if...
|
||||
// - we know the image is definitely normalized, and
|
||||
// - we're doing some color space conversion, and
|
||||
// - sRGB curves are involved,
|
||||
// then we can use slightly faster math that doesn't work well outside [0,1].
|
||||
bool src_is_normalized = SkColorTypeIsNormalized(info.colorType());
|
||||
|
||||
// Bicubic filtering naturally produces out of range values on both sides of [0,1].
|
||||
if (quality > kLow_SkFilterQuality) {
|
||||
// Bicubic filtering naturally produces out of range values on both sides.
|
||||
p->append(SkRasterPipeline::clamp_0);
|
||||
p->append(fClampAsIfUnpremul ? SkRasterPipeline::clamp_1
|
||||
: SkRasterPipeline::clamp_a);
|
||||
src_is_normalized = true;
|
||||
}
|
||||
|
||||
// Transform color space and alpha type to match shader convention (dst CS, premul alpha).
|
||||
alloc->make<SkColorSpaceXformSteps>(cs, at,
|
||||
rec.fDstCS, kPremul_SkAlphaType)
|
||||
->apply(p, src_is_normalized);
|
||||
if (rec.fDstCS) {
|
||||
// If color managed, convert from premul source all the way to premul dst color space.
|
||||
auto srcCS = info.colorSpace();
|
||||
if (!srcCS || info.colorType() == kAlpha_8_SkColorType) {
|
||||
// We treat untagged images as sRGB.
|
||||
// A8 images get their r,g,b from the paint color, so they're also sRGB.
|
||||
srcCS = sk_srgb_singleton();
|
||||
}
|
||||
alloc->make<SkColorSpaceXformSteps>(srcCS , kPremul_SkAlphaType,
|
||||
rec.fDstCS, kPremul_SkAlphaType)
|
||||
->apply(p, info.colorType());
|
||||
}
|
||||
|
||||
return true;
|
||||
};
|
||||
|
Loading…
Reference in New Issue
Block a user