From f421685918c5976eecc4459a787f5a45bdb1e0a3 Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Thu, 11 Oct 2018 17:47:17 +0000 Subject: [PATCH] Revert "Stop using color space xform canvas in picture image generator" This reverts commit 031ca213e0921cf4d2467aa41d29c75d46092009. Reason for revert: breaking angle surfacetest unit test Original change's description: > Stop using color space xform canvas in picture image generator > > Direct rasterization should produce similar (or identical) results. > See: https://chromium-review.googlesource.com/c/chromium/src/+/1273815 > > Bug: skia: > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I252ec5d5ecb19704f33c0f6cb65d12c2ec927c1d > Reviewed-on: https://skia-review.googlesource.com/c/161140 > Commit-Queue: Brian Osman > Reviewed-by: Mike Klein TBR=mtklein@google.com,brianosman@google.com Change-Id: Ib5667f0d315c4f2877c1f0c38e9c62d37335d511 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia: Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://skia-review.googlesource.com/c/161560 Reviewed-by: Greg Daniel Commit-Queue: Greg Daniel --- src/core/SkPictureImageGenerator.cpp | 8 -------- src/image/SkSurface_Gpu.cpp | 17 +++++++++++++++-- src/image/SkSurface_Raster.cpp | 27 ++++++++++++++++++++++++++- tests/SurfaceTest.cpp | 4 ++-- 4 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/core/SkPictureImageGenerator.cpp b/src/core/SkPictureImageGenerator.cpp index fa55538971..287d564c43 100644 --- a/src/core/SkPictureImageGenerator.cpp +++ b/src/core/SkPictureImageGenerator.cpp @@ -53,11 +53,7 @@ SkPictureImageGenerator::SkPictureImageGenerator(const SkImageInfo& info, sk_sp< bool SkPictureImageGenerator::onGetPixels(const SkImageInfo& info, void* pixels, size_t rowBytes, const Options& opts) { // TODO: Stop using xform canvas and simplify this code once rasterization works the same way -#ifdef SK_LEGACY_XFORM_CANVAS_IN_PICTURE_IMAGES bool useXformCanvas = /* kIgnore == behavior && */ info.colorSpace(); -#else - bool useXformCanvas = false; -#endif SkSurfaceProps props(0, kUnknown_SkPixelGeometry); SkImageInfo canvasInfo = useXformCanvas ? info.makeColorSpace(nullptr) : info; @@ -102,11 +98,7 @@ sk_sp SkPictureImageGenerator::onGenerateTexture( GrContext* ctx, const SkImageInfo& info, const SkIPoint& origin, bool willNeedMipMaps) { SkASSERT(ctx); // TODO: Stop using xform canvas and simplify this code once rasterization works the same way -#ifdef SK_LEGACY_XFORM_CANVAS_IN_PICTURE_IMAGES bool useXformCanvas = /* behavior == kIgnore && */ info.colorSpace(); -#else - bool useXformCanvas = false; -#endif // // TODO: respect the usage, by possibly creating a different (pow2) surface diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 9c04ac2e8e..9bcd90ac92 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/src/image/SkSurface_Gpu.cpp @@ -262,7 +262,15 @@ bool SkSurface_Gpu::onDraw(const SkDeferredDisplayList* ddl) { /////////////////////////////////////////////////////////////////////////////// bool SkSurface_Gpu::Valid(const SkImageInfo& info) { - return true; + switch (info.colorType()) { + case kRGBA_F16_SkColorType: + case kRGBA_F32_SkColorType: + case kRGBA_8888_SkColorType: + case kBGRA_8888_SkColorType: + return true; + default: + return !info.colorSpace(); + } } bool SkSurface_Gpu::Valid(const GrCaps* caps, GrPixelConfig config, SkColorSpace* colorSpace) { @@ -270,8 +278,13 @@ bool SkSurface_Gpu::Valid(const GrCaps* caps, GrPixelConfig config, SkColorSpace case kSRGBA_8888_GrPixelConfig: case kSBGRA_8888_GrPixelConfig: return caps->srgbSupport(); - default: + case kRGBA_half_GrPixelConfig: + case kRGBA_float_GrPixelConfig: + case kRGBA_8888_GrPixelConfig: + case kBGRA_8888_GrPixelConfig: return true; + default: + return !colorSpace; } } diff --git a/src/image/SkSurface_Raster.cpp b/src/image/SkSurface_Raster.cpp index 19078239d4..4076dff3a0 100644 --- a/src/image/SkSurface_Raster.cpp +++ b/src/image/SkSurface_Raster.cpp @@ -42,6 +42,32 @@ bool SkSurfaceValidateRasterInfo(const SkImageInfo& info, size_t rowBytes) { return false; } + static const size_t kMaxTotalSize = SK_MaxS32; + + // TODO(mtklein,brianosman): revisit all these color space decisions + switch (info.colorType()) { + case kAlpha_8_SkColorType: + case kGray_8_SkColorType: + case kRGB_565_SkColorType: + case kARGB_4444_SkColorType: + case kRGB_888x_SkColorType: + case kRGBA_1010102_SkColorType: + case kRGB_101010x_SkColorType: + if (info.colorSpace()) { + return false; + } + break; + + case kRGBA_8888_SkColorType: + case kBGRA_8888_SkColorType: + break; + case kRGBA_F16_SkColorType: + case kRGBA_F32_SkColorType: + break; + default: + return false; + } + if (kIgnoreRowBytesValue == rowBytes) { return true; } @@ -59,7 +85,6 @@ bool SkSurfaceValidateRasterInfo(const SkImageInfo& info, size_t rowBytes) { } uint64_t size = sk_64_mul(info.height(), rowBytes); - static const size_t kMaxTotalSize = SK_MaxS32; if (size > kMaxTotalSize) { return false; } diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp index df2add1198..71c437ce8a 100644 --- a/tests/SurfaceTest.cpp +++ b/tests/SurfaceTest.cpp @@ -941,8 +941,8 @@ static void test_surface_creation_and_snapshot_with_color_space( { kRGBA_F32_SkColorType, linearColorSpace, supportsF32, "F32-linear" }, { kRGBA_F32_SkColorType, srgbColorSpace, supportsF32, "F32-srgb" }, { kRGBA_F32_SkColorType, oddColorSpace, supportsF32, "F32-odd" }, - { kRGB_565_SkColorType, srgbColorSpace, true, "565-srgb" }, - { kAlpha_8_SkColorType, srgbColorSpace, true, "A8-srgb" }, + { kRGB_565_SkColorType, srgbColorSpace, false, "565-srgb" }, + { kAlpha_8_SkColorType, srgbColorSpace, false, "A8-srgb" }, { kRGBA_1010102_SkColorType, nullptr, supports1010102, "1010102-nullptr" }, };