AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Explicitly fail read/writePixels in invalid color space scenarios

Browse Source 
It's not well defined what to do when moving from a nullptr color space to
a tagged destination (drawing, reading, writing, etc...). In these
scenarios, at least, we can choose to disallow the operation (rather than
produce an unexpected or inconsistent result).

BUG=skia:

Change-Id: I033b23c6f2bb00664efc8fdab1b3f52053d77695
Reviewed-on: https://skia-review.googlesource.com/6600
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
This commit is contained in:
Brian Osman 2017-01-05 10:41:39 -05:00 committed by Skia Commit-Bot
parent 08d57e6ae6
commit efcc41805b
3 changed files with 48 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/gpu/GrContext.cpp
View File

@ -278,6 +278,11 @@ bool GrContext::writeSurfacePixels(GrSurface* surface, SkColorSpace* dstColorSpa
return false;
}
// We don't allow writing to a color space tagged destination if the source isn't tagged
if (dstColorSpace && !srcColorSpace) {
return false;
}
GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference;
// Don't prefer to draw for the conversion (and thereby access a texture from the cache) when
// we've already determined that there isn't a roundtrip preserving conversion processor pair.
@ -427,6 +432,11 @@ bool GrContext::readSurfacePixels(GrSurface* src, SkColorSpace* srcColorSpace,
return false;
}
// We don't allow reading to a color space tagged destination if the source isn't tagged
if (dstColorSpace && !srcColorSpace) {
return false;
}
GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference;
// Don't prefer to draw for the conversion (and thereby access a texture from the cache) when
// we've already determined that there isn't a roundtrip preserving conversion processor pair.

3
tests/ImageTest.cpp
View File

@ -45,8 +45,7 @@ static void assert_equal(skiatest::Reporter* reporter, SkImage* a, const SkIRect
// see https://bug.skia.org/3965
//REPORTER_ASSERT(reporter, a->isOpaque() == b->isOpaque());
// The codecs may have given us back F16, we can't read from F16 raster to N32, only S32.
SkImageInfo info = SkImageInfo::MakeS32(widthA, heightA, a->alphaType());
SkImageInfo info = SkImageInfo::MakeN32(widthA, heightA, a->alphaType());
SkAutoPixmapStorage pmapA, pmapB;
pmapA.alloc(info);
pmapB.alloc(info);

37
tests/ReadPixelsTest.cpp
View File

@ -487,3 +487,40 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) {
}
}
#endif
#if SK_SUPPORT_GPU
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixelsColorSpaceVariants_Gpu, reporter, ctxInfo) {
// Create surfaces with and without an attached color space
sk_sp<SkColorSpace> srgbColorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
SkImageInfo srgbInfo = SkImageInfo::MakeS32(DEV_W, DEV_H, kPremul_SkAlphaType);
SkImageInfo legacyInfo = srgbInfo.makeColorSpace(nullptr);
sk_sp<SkSurface> srgbSurface = SkSurface::MakeRenderTarget(ctxInfo.grContext(), SkBudgeted::kNo,
srgbInfo);
sk_sp<SkSurface> legacySurface = SkSurface::MakeRenderTarget(ctxInfo.grContext(),
SkBudgeted::kNo, legacyInfo);
SkCanvas* srgbCanvas = srgbSurface->getCanvas();
SkCanvas* legacyCanvas = legacySurface->getCanvas();
struct {
SkCanvas* fCanvas;
const SkImageInfo& fBmpInfo;
bool fExpectSuccess;
} kTestConfigs[] ={
// Both kinds of surface should be able to read into a legacy destination
{ srgbCanvas, legacyInfo, true },
{ legacyCanvas, legacyInfo, true },
// Tagged surface should be able to read into tagged destination
{ srgbCanvas, srgbInfo, true },
// Legacy surface shouldn't read into tagged destination
{ legacyCanvas, srgbInfo, false },
};
for (auto testConfig : kTestConfigs) {
SkBitmap bmp;
bmp.setInfo(testConfig.fBmpInfo);
bool result = testConfig.fCanvas->readPixels(&bmp, 0, 0);
REPORTER_ASSERT(reporter, result == testConfig.fExpectSuccess);
}
}
#endif