Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 4311f19

Browse files
Mike KleinSkia Commit-Bot
authored andcommitted
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>
1 parent 82d4970 commit 4311f19

File tree

2 files changed

+20
-23
lines changed

2 files changed

+20
-23
lines changed

src/core/SkColorSpaceXformSteps.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,6 @@ struct SkColorSpaceXformSteps {
4343
void apply(float rgba[4]) const;
4444
void apply(SkRasterPipeline*, bool src_is_normalized) const;
4545

46-
void apply(SkRasterPipeline* p, SkColorType srcCT) const {
47-
return this->apply(p, SkColorTypeIsNormalized(srcCT));
48-
}
49-
5046
Flags flags;
5147

5248
bool srcTF_is_sRGB,

src/shaders/SkImageShader.cpp

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -479,36 +479,37 @@ bool SkImageShader::doStages(const SkStageRec& rec, SkImageStageUpdater* updater
479479
};
480480

481481
auto append_misc = [&] {
482-
// TODO: if ref.fDstCS isn't null, we'll premul here then immediately unpremul
483-
// to do the color space transformation. Might be possible to streamline.
482+
SkColorSpace* cs = info.colorSpace();
483+
SkAlphaType at = info.alphaType();
484+
485+
// Color for A8 images comes from the paint. TODO: all alpha formats? maybe none?
484486
if (info.colorType() == kAlpha_8_SkColorType) {
485-
// The color for A8 images comes from the (sRGB) paint color.
486487
p->append_set_rgb(alloc, rec.fPaint.getColor4f());
487488
p->append(SkRasterPipeline::premul);
488-
} else if (info.alphaType() == kUnpremul_SkAlphaType) {
489-
// Convert unpremul images to premul before we carry on with the rest of the pipeline.
490-
p->append(SkRasterPipeline::premul);
489+
cs = sk_srgb_singleton(); // TODO: transform to dstCS here instead of at runtime?
490+
at = kPremul_SkAlphaType;
491491
}
492492

493+
// This is an unessential optimization... it's logically safe to set this to false.
494+
// But if...
495+
// - we know the image is definitely normalized, and
496+
// - we're doing some color space conversion, and
497+
// - sRGB curves are involved,
498+
// then we can use slightly faster math that doesn't work well outside [0,1].
499+
bool src_is_normalized = SkColorTypeIsNormalized(info.colorType());
500+
501+
// Bicubic filtering naturally produces out of range values on both sides of [0,1].
493502
if (quality > kLow_SkFilterQuality) {
494-
// Bicubic filtering naturally produces out of range values on both sides.
495503
p->append(SkRasterPipeline::clamp_0);
496504
p->append(fClampAsIfUnpremul ? SkRasterPipeline::clamp_1
497505
: SkRasterPipeline::clamp_a);
506+
src_is_normalized = true;
498507
}
499508

500-
if (rec.fDstCS) {
501-
// If color managed, convert from premul source all the way to premul dst color space.
502-
auto srcCS = info.colorSpace();
503-
if (!srcCS || info.colorType() == kAlpha_8_SkColorType) {
504-
// We treat untagged images as sRGB.
505-
// A8 images get their r,g,b from the paint color, so they're also sRGB.
506-
srcCS = sk_srgb_singleton();
507-
}
508-
alloc->make<SkColorSpaceXformSteps>(srcCS , kPremul_SkAlphaType,
509-
rec.fDstCS, kPremul_SkAlphaType)
510-
->apply(p, info.colorType());
511-
}
509+
// Transform color space and alpha type to match shader convention (dst CS, premul alpha).
510+
alloc->make<SkColorSpaceXformSteps>(cs, at,
511+
rec.fDstCS, kPremul_SkAlphaType)
512+
->apply(p, src_is_normalized);
512513

513514
return true;
514515
};

0 commit comments

Comments
 (0)