From 9679612d6b34d8d7956a5ebccae15f7e24412a99 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Tue, 19 Jan 2021 12:11:07 -0500 Subject: [PATCH] Reland "Test all YUVA image factories with different encoded origins." This reverts commit f1650efc557e75520f810d21f5f574f1ece005fe. Reason for revert: fix for cpu/ddl configs Original change's description: > Revert "Test all YUVA image factories with different encoded origins." > > This reverts commit 2ba80af000b7efa1c54616bd12edc627e5cb45af. > > Reason for revert: new test fails ddl and cpu configs on imggen > > Original change's description: > > Test all YUVA image factories with different encoded origins. > > > > Now that SkImage_GpuYUVA stores a GrYUVATextureProxies it supports > > encoded origins. > > > > Modify wacky_yuv_format GMs to use different origins and remove > > restriction in SkImage::MakeFromYUVAPixmaps. > > > > Bug: skia:10632 > > Change-Id: I02477d592b7baba164944d629eeac48223698c10 > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353623 > > Reviewed-by: Jim Van Verth > > Commit-Queue: Brian Salomon > > TBR=jvanverth@google.com,bsalomon@google.com > > Change-Id: If909ee4769cc1c74e1682a5e2870ec85a83f65c5 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: skia:10632 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354661 > Reviewed-by: Brian Salomon > Commit-Queue: Brian Salomon TBR=jvanverth@google.com,bsalomon@google.com # Not skipping CQ checks because this is a reland. Bug: skia:10632 Change-Id: Iafe79ab5b3ce0ff9e3a4007e5d8fbc44edded196 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/355630 Reviewed-by: Brian Salomon Reviewed-by: Jim Van Verth Commit-Queue: Brian Salomon --- gm/wacky_yuv_formats.cpp | 89 +++++++++++++++++++++++---------- src/image/SkImage_GpuYUVA.cpp | 12 ----- tools/DDLPromiseImageHelper.cpp | 3 +- tools/DDLPromiseImageHelper.h | 5 +- tools/gpu/YUVUtils.cpp | 36 ++++++++----- 5 files changed, 87 insertions(+), 58 deletions(-) diff --git a/gm/wacky_yuv_formats.cpp b/gm/wacky_yuv_formats.cpp index 8ec7160d2c..5eb6626aec 100644 --- a/gm/wacky_yuv_formats.cpp +++ b/gm/wacky_yuv_formats.cpp @@ -114,7 +114,7 @@ static bool has_alpha_channel(YUVFormat format) { class YUVAPlanarConfig { public: - YUVAPlanarConfig(YUVFormat format, bool opaque) { + YUVAPlanarConfig(YUVFormat format, bool opaque, SkEncodedOrigin origin) : fOrigin(origin) { switch (format) { case kP016_YUVFormat: case kP010_YUVFormat: @@ -187,13 +187,14 @@ public: private: SkYUVAInfo::PlaneConfig fPlaneConfig; SkYUVAInfo::Subsampling fSubsampling; + SkEncodedOrigin fOrigin; }; SkYUVAPixmaps YUVAPlanarConfig::makeYUVAPixmaps(SkISize dimensions, SkYUVColorSpace yuvColorSpace, const SkBitmap bitmaps[], int numBitmaps) const { - SkYUVAInfo info(dimensions, fPlaneConfig, fSubsampling, yuvColorSpace); + SkYUVAInfo info(dimensions, fPlaneConfig, fSubsampling, yuvColorSpace, fOrigin); SkPixmap pmaps[SkYUVAInfo::kMaxPlanes]; int n = info.numPlanes(); if (numBitmaps < n) { @@ -321,9 +322,11 @@ static SkPath create_splat(const SkPoint& o, SkScalar innerRadius, SkScalar oute static SkBitmap make_bitmap(SkColorType colorType, const SkPath& path, const SkTDArray& circles, bool opaque, bool padWithRed) { - const SkColor kGreen = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 178, 240, 104)); - const SkColor kBlue = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 173, 167, 252)); - const SkColor kYellow = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 255, 221, 117)); + const SkColor kGreen = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 178, 240, 104)); + const SkColor kBlue = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 173, 167, 252)); + const SkColor kYellow = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 255, 221, 117)); + const SkColor kMagenta = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 255, 60, 217)); + const SkColor kCyan = ToolUtils::color_to_565(SkColorSetARGB(0xFF, 45, 237, 205)); int widthHeight = kTileWidthHeight + (padWithRed ? 2 * kSubsetPadding : 0); @@ -349,9 +352,16 @@ static SkBitmap make_bitmap(SkColorType colorType, const SkPath& path, canvas->drawPath(path, paint); - paint.setColor(opaque ? kYellow : SK_ColorTRANSPARENT); paint.setBlendMode(SkBlendMode::kSrc); for (int i = 0; i < circles.count(); ++i) { + SkColor color; + switch (i % 3) { + case 0: color = kYellow; break; + case 1: color = kMagenta; break; + default: color = kCyan; break; + } + paint.setColor(color); + paint.setAlpha(opaque ? 0xFF : 0x40); SkRect r = circles[i]; r.inset(r.width()/4, r.height()/4); canvas->drawOval(r, paint); @@ -371,38 +381,53 @@ static void convert_rgba_to_yuva(const float mtx[20], SkColor col, uint8_t yuv[4 yuv[3] = SkColorGetA(col); } -static void extract_planes(const SkBitmap& bm, SkYUVColorSpace yuvColorSpace, PlaneData* planes) { - if (kIdentity_SkYUVColorSpace == yuvColorSpace) { +static void extract_planes(const SkBitmap& origBM, + SkYUVColorSpace yuvColorSpace, + SkEncodedOrigin origin, + PlaneData* planes) { + SkImageInfo ii = origBM.info(); + if (SkEncodedOriginSwapsWidthHeight(origin)) { + ii = ii.makeWH(ii.height(), ii.width()); + } + SkBitmap orientedBM; + orientedBM.allocPixels(ii); + SkCanvas canvas(orientedBM); + SkMatrix matrix = SkEncodedOriginToMatrix(origin, origBM.width(), origBM.height()); + SkAssertResult(matrix.invert(&matrix)); + canvas.concat(matrix); + canvas.drawBitmap(origBM, 0, 0); + + if (yuvColorSpace == kIdentity_SkYUVColorSpace) { // To test the identity color space we use JPEG YUV planes yuvColorSpace = kJPEG_SkYUVColorSpace; } - SkASSERT(!(bm.width() % 2)); - SkASSERT(!(bm.height() % 2)); + SkASSERT(!(ii.width() % 2)); + SkASSERT(!(ii.height() % 2)); planes->fYFull.allocPixels( - SkImageInfo::Make(bm.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); + SkImageInfo::Make(ii.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); planes->fUFull.allocPixels( - SkImageInfo::Make(bm.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); + SkImageInfo::Make(ii.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); planes->fVFull.allocPixels( - SkImageInfo::Make(bm.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); - planes->fAFull.allocPixels(SkImageInfo::MakeA8(bm.width(), bm.height())); - planes->fUQuarter.allocPixels(SkImageInfo::Make(bm.width()/2, bm.height()/2, + SkImageInfo::Make(ii.dimensions(), kGray_8_SkColorType, kUnpremul_SkAlphaType)); + planes->fAFull.allocPixels(SkImageInfo::MakeA8(ii.dimensions())); + planes->fUQuarter.allocPixels(SkImageInfo::Make(ii.width()/2, ii.height()/2, kGray_8_SkColorType, kUnpremul_SkAlphaType)); - planes->fVQuarter.allocPixels(SkImageInfo::Make(bm.width()/2, bm.height()/2, + planes->fVQuarter.allocPixels(SkImageInfo::Make(ii.width()/2, ii.height()/2, kGray_8_SkColorType, kUnpremul_SkAlphaType)); planes->fFull.allocPixels( - SkImageInfo::Make(bm.dimensions(), kRGBA_F32_SkColorType, kUnpremul_SkAlphaType)); - planes->fQuarter.allocPixels(SkImageInfo::Make(bm.width()/2, bm.height()/2, + SkImageInfo::Make(ii.dimensions(), kRGBA_F32_SkColorType, kUnpremul_SkAlphaType)); + planes->fQuarter.allocPixels(SkImageInfo::Make(ii.width()/2, ii.height()/2, kRGBA_F32_SkColorType, kUnpremul_SkAlphaType)); float mtx[20]; SkColorMatrix_RGB2YUV(yuvColorSpace, mtx); SkColor4f* dst = (SkColor4f *) planes->fFull.getAddr(0, 0); - for (int y = 0; y < bm.height(); ++y) { - for (int x = 0; x < bm.width(); ++x) { - SkColor col = bm.getColor(x, y); + for (int y = 0; y < orientedBM.height(); ++y) { + for (int x = 0; x < orientedBM.width(); ++x) { + SkColor col = orientedBM.getColor(x, y); uint8_t yuva[4]; @@ -423,8 +448,8 @@ static void extract_planes(const SkBitmap& bm, SkYUVColorSpace yuvColorSpace, Pl } dst = (SkColor4f *) planes->fQuarter.getAddr(0, 0); - for (int y = 0; y < bm.height()/2; ++y) { - for (int x = 0; x < bm.width()/2; ++x) { + for (int y = 0; y < orientedBM.height()/2; ++y) { + for (int x = 0; x < orientedBM.width()/2; ++x) { uint32_t yAccum = 0, uAccum = 0, vAccum = 0, aAccum = 0; yAccum += *planes->fYFull.getAddr8(2*x, 2*y); @@ -801,17 +826,23 @@ protected: } bool createImages(GrDirectContext* dContext) { + int origin = 0; for (bool opaque : { false, true }) { for (int cs = kJPEG_SkYUVColorSpace; cs <= kLastEnum_SkYUVColorSpace; ++cs) { PlaneData planes; - extract_planes(fOriginalBMs[opaque], (SkYUVColorSpace) cs, &planes); + extract_planes(fOriginalBMs[opaque], + static_cast(cs), + static_cast(origin + 1), // valid origins are 1...8 + &planes); for (int f = kP016_YUVFormat; f <= kLast_YUVFormat; ++f) { auto format = static_cast(f); SkBitmap resultBMs[4]; int numPlanes = create_YUV(planes, format, resultBMs, opaque); - const YUVAPlanarConfig planarConfig(format, opaque); + const YUVAPlanarConfig planarConfig(format, + opaque, + static_cast(origin + 1)); SkYUVAPixmaps pixmaps = planarConfig.makeYUVAPixmaps(fOriginalBMs[opaque].dimensions(), static_cast(cs), @@ -821,6 +852,7 @@ protected: fImages[opaque][cs][format] = lazyYUV->refImage(dContext, fImageType); } + origin = (origin + 1) % 8; } } @@ -998,13 +1030,16 @@ protected: bool createImages(GrDirectContext* context) { for (bool opaque : { false, true }) { PlaneData planes; - extract_planes(fOriginalBMs[opaque], kJPEG_SkYUVColorSpace, &planes); + extract_planes(fOriginalBMs[opaque], + kJPEG_SkYUVColorSpace, + kTopLeft_SkEncodedOrigin, + &planes); SkBitmap resultBMs[4]; create_YUV(planes, kAYUV_YUVFormat, resultBMs, opaque); - YUVAPlanarConfig planarConfig(kAYUV_YUVFormat, opaque); + YUVAPlanarConfig planarConfig(kAYUV_YUVFormat, opaque, kTopLeft_SkEncodedOrigin); auto yuvaPixmaps = planarConfig.makeYUVAPixmaps(fOriginalBMs[opaque].dimensions(), kJPEG_Full_SkYUVColorSpace, diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp index 8d95a3b744..e5b41aec30 100644 --- a/src/image/SkImage_GpuYUVA.cpp +++ b/src/image/SkImage_GpuYUVA.cpp @@ -273,11 +273,6 @@ sk_sp SkImage::MakeFromYUVAPixmaps(GrRecordingContext* context, return nullptr; } - // SkImage_GpuYUVA doesn't yet support different encoded origins. - if (pixmaps.yuvaInfo().origin() != kTopLeft_SkEncodedOrigin) { - return nullptr; - } - if (!context->priv().caps()->mipmapSupport()) { buildMips = GrMipMapped::kNo; } @@ -361,13 +356,6 @@ sk_sp SkImage_GpuYUVA::MakePromiseYUVATexture( releaseHelpers[i] = GrRefCntedCallback::Make(textureReleaseProc, textureContexts[i]); } - if (yuvaBackendTextureInfo.yuvaInfo().origin() != SkEncodedOrigin::kDefault_SkEncodedOrigin) { - // SkImage_GpuYUVA does not support this yet. This will get removed - // when the old APIs are gone and we only have to support YUVA configs described by - // SkYUVAInfo. Fix with skbug.com/10632. - return nullptr; - } - if (!context) { return nullptr; } diff --git a/tools/DDLPromiseImageHelper.cpp b/tools/DDLPromiseImageHelper.cpp index 3acb3412c4..c4ff918680 100644 --- a/tools/DDLPromiseImageHelper.cpp +++ b/tools/DDLPromiseImageHelper.cpp @@ -69,8 +69,7 @@ void DDLPromiseImageHelper::PromiseImageInfo::setMipLevels(const SkBitmap& baseL /////////////////////////////////////////////////////////////////////////////////////////////////// PromiseImageCallbackContext::~PromiseImageCallbackContext() { - // See comment in release() about YUVA image creation failures. - // SkASSERT(fDoneCnt == fNumImages);1 + SkASSERT(fDoneCnt == fNumImages); SkASSERT(!fTotalFulfills || fDoneCnt); if (fPromiseImageTexture) { diff --git a/tools/DDLPromiseImageHelper.h b/tools/DDLPromiseImageHelper.h index 8306d402ae..cd65a7f52a 100644 --- a/tools/DDLPromiseImageHelper.h +++ b/tools/DDLPromiseImageHelper.h @@ -51,10 +51,7 @@ public: void release() { ++fDoneCnt; - // YUVA Images can fail to create if they have a non-default encoded origin. This will be - // fixed when skbug.com/10632 is finished. This assert fires because done is called on - // failure and we never bumped fNumImages. - // SkASSERT(fDoneCnt <= fNumImages); + SkASSERT(fDoneCnt <= fNumImages); } void wasAddedToImage() { fNumImages++; } diff --git a/tools/gpu/YUVUtils.cpp b/tools/gpu/YUVUtils.cpp index dbbbd2bd61..9344998325 100644 --- a/tools/gpu/YUVUtils.cpp +++ b/tools/gpu/YUVUtils.cpp @@ -33,11 +33,11 @@ static SkPMColor convert_yuva_to_rgba(const float mtx[20], uint8_t yuva[4]) { return SkPremultiplyARGBInline(a, r, g, b); } -static uint8_t look_up(float x1, float y1, const SkPixmap& pmap, SkColorChannel channel) { - SkASSERT(x1 > 0 && x1 < 1.0f); - SkASSERT(y1 > 0 && y1 < 1.0f); - int x = SkScalarFloorToInt(x1 * pmap.width()); - int y = SkScalarFloorToInt(y1 * pmap.height()); +static uint8_t look_up(SkPoint normPt, const SkPixmap& pmap, SkColorChannel channel) { + SkASSERT(normPt.x() > 0 && normPt.x() < 1.0f); + SkASSERT(normPt.y() > 0 && normPt.y() < 1.0f); + int x = SkScalarFloorToInt(normPt.x() * pmap.width()); + int y = SkScalarFloorToInt(normPt.y() * pmap.height()); auto ii = pmap.info().makeColorType(kRGBA_8888_SkColorType).makeWH(1, 1); uint32_t pixel; @@ -69,10 +69,21 @@ protected: SkYUVAInfo::YUVALocations yuvaLocations = fPixmaps.toYUVALocations(); SkASSERT(SkYUVAInfo::YUVALocation::AreValidLocations(yuvaLocations)); + SkMatrix om = fPixmaps.yuvaInfo().originMatrix(); + SkAssertResult(om.invert(&om)); + float normX = 1.f/info.width(); + float normY = 1.f/info.height(); + if (SkEncodedOriginSwapsWidthHeight(fPixmaps.yuvaInfo().origin())) { + using std::swap; + swap(normX, normY); + } for (int y = 0; y < info.height(); ++y) { for (int x = 0; x < info.width(); ++x) { - float x1 = (x + 0.5f) / info.width(); - float y1 = (y + 0.5f) / info.height(); + SkPoint xy1 {(x + 0.5f), + (y + 0.5f)}; + xy1 = om.mapPoint(xy1); + xy1.fX *= normX; + xy1.fY *= normY; uint8_t yuva[4] = {0, 0, 0, 255}; @@ -80,13 +91,12 @@ protected: SkYUVAInfo::YUVAChannels::kU, SkYUVAInfo::YUVAChannels::kV}) { const auto& pmap = fPixmaps.plane(yuvaLocations[c].fPlane); - yuva[c] = look_up(x1, y1, pmap, yuvaLocations[c].fChannel); + yuva[c] = look_up(xy1, pmap, yuvaLocations[c].fChannel); } - if (yuvaLocations[SkYUVAInfo::YUVAChannels::kA].fPlane >= 0) { - const auto& pmap = - fPixmaps.plane(yuvaLocations[SkYUVAInfo::YUVAChannels::kA].fPlane); - yuva[3] = look_up( - x1, y1, pmap, yuvaLocations[SkYUVAInfo::YUVAChannels::kA].fChannel); + auto [aPlane, aChan] = yuvaLocations[SkYUVAInfo::YUVAChannels::kA]; + if (aPlane >= 0) { + const auto& pmap = fPixmaps.plane(aPlane); + yuva[3] = look_up(xy1, pmap, aChan); } // Making premul here.