Add a mutex to GrContext::readSurfacePixels to protect against multiple CPU raster threads accessing the same GrContext to read back GPU input data

BUG=chromium:524717

TBR=reed@google.com

Committed: https://skia.googlesource.com/skia/+/eb662bc407cec0585a821946fef123102cae64db

Review URL: https://codereview.chromium.org/1329313002
This commit is contained in:
bsalomon 2015-09-10 08:12:46 -07:00 committed by Commit bot
parent 059dffae80
commit 6c6f65885b
2 changed files with 45 additions and 14 deletions

View File

@ -15,6 +15,7 @@
#include "GrRenderTarget.h"
#include "GrTextureProvider.h"
#include "SkMatrix.h"
#include "SkMutex.h"
#include "SkPathEffect.h"
#include "SkTypes.h"
@ -380,6 +381,18 @@ private:
bool fDidTestPMConversions;
int fPMToUPMConversion;
int fUPMToPMConversion;
// The sw backend may call GrContext::readSurfacePixels on multiple threads
// We may transfer the responsibilty for using a mutex to the sw backend
// when there are fewer code paths that lead to a readSurfacePixels call
// from the sw backend. readSurfacePixels is reentrant in one case - when performing
// the PM conversions test. To handle this we do the PM conversions test outside
// of fReadPixelsMutex and use a separate mutex to guard it. When it re-enters
// readSurfacePixels it will grab fReadPixelsMutex and release it before the outer
// readSurfacePixels proceeds to grab it.
// TODO: Stop pretending to make GrContext thread-safe for sw rasterization and provide
// a mechanism to make a SkPicture safe for multithreaded sw rasterization.
SkMutex fReadPixelsMutex;
SkMutex fTestPMConversionsMutex;
struct CleanUpData {
PFCleanUpFunc fFunc;
@ -446,9 +459,13 @@ private:
* return NULL.
*/
const GrFragmentProcessor* createPMToUPMEffect(GrProcessorDataManager*, GrTexture*,
bool swapRAndB, const SkMatrix&);
bool swapRAndB, const SkMatrix&) const;
const GrFragmentProcessor* createUPMToPMEffect(GrProcessorDataManager*, GrTexture*,
bool swapRAndB, const SkMatrix&);
bool swapRAndB, const SkMatrix&) const;
/** Called before either of the above two functions to determine the appropriate fragment
processors for conversions. This must be called by readSurfacePixels befor a mutex is taken,
since testingvPM conversions itself will call readSurfacePixels */
void testPMConversionsIfNecessary(uint32_t flags);
/** Returns true if we've already determined that createPMtoUPMEffect and createUPMToPMEffect
will fail. In such cases fall back to SW conversion. */
bool didFailPMUPMConversionTest() const;

View File

@ -329,6 +329,10 @@ bool GrContext::writeSurfacePixels(GrSurface* surface,
GrPixelConfig srcConfig, const void* buffer, size_t rowBytes,
uint32_t pixelOpsFlags) {
RETURN_FALSE_IF_ABANDONED
ASSERT_OWNED_RESOURCE(surface);
SkASSERT(surface);
this->testPMConversionsIfNecessary(pixelOpsFlags);
// Trim the params here so that if we wind up making a temporary surface it can be as small as
// necessary and because GrGpu::getWritePixelsInfo requires it.
@ -461,6 +465,9 @@ bool GrContext::readSurfacePixels(GrSurface* src,
ASSERT_OWNED_RESOURCE(src);
SkASSERT(src);
this->testPMConversionsIfNecessary(flags);
SkAutoMutexAcquire ama(fReadPixelsMutex);
// Adjust the params so that if we wind up using an intermediate surface we've already done
// all the trimming and the temporary can be the min size required.
if (!GrSurfacePriv::AdjustReadPixelParams(src->width(), src->height(),
@ -694,14 +701,22 @@ void test_pm_conversions(GrContext* ctx, int* pmToUPMValue, int* upmToPMValue) {
}
}
void GrContext::testPMConversionsIfNecessary(uint32_t flags) {
if (SkToBool(kUnpremul_PixelOpsFlag & flags)) {
SkAutoMutexAcquire ama(fTestPMConversionsMutex);
if (!fDidTestPMConversions) {
test_pm_conversions(this, &fPMToUPMConversion, &fUPMToPMConversion);
fDidTestPMConversions = true;
}
}
}
const GrFragmentProcessor* GrContext::createPMToUPMEffect(GrProcessorDataManager* procDataManager,
GrTexture* texture,
bool swapRAndB,
const SkMatrix& matrix) {
if (!fDidTestPMConversions) {
test_pm_conversions(this, &fPMToUPMConversion, &fUPMToPMConversion);
fDidTestPMConversions = true;
}
const SkMatrix& matrix) const {
// We should have already called this->testPMConversionsIfNecessary().
SkASSERT(fDidTestPMConversions);
GrConfigConversionEffect::PMConversion pmToUPM =
static_cast<GrConfigConversionEffect::PMConversion>(fPMToUPMConversion);
if (GrConfigConversionEffect::kNone_PMConversion != pmToUPM) {
@ -715,11 +730,9 @@ const GrFragmentProcessor* GrContext::createPMToUPMEffect(GrProcessorDataManager
const GrFragmentProcessor* GrContext::createUPMToPMEffect(GrProcessorDataManager* procDataManager,
GrTexture* texture,
bool swapRAndB,
const SkMatrix& matrix) {
if (!fDidTestPMConversions) {
test_pm_conversions(this, &fPMToUPMConversion, &fUPMToPMConversion);
fDidTestPMConversions = true;
}
const SkMatrix& matrix) const {
// We should have already called this->testPMConversionsIfNecessary().
SkASSERT(fDidTestPMConversions);
GrConfigConversionEffect::PMConversion upmToPM =
static_cast<GrConfigConversionEffect::PMConversion>(fUPMToPMConversion);
if (GrConfigConversionEffect::kNone_PMConversion != upmToPM) {
@ -731,9 +744,10 @@ const GrFragmentProcessor* GrContext::createUPMToPMEffect(GrProcessorDataManager
}
bool GrContext::didFailPMUPMConversionTest() const {
// We should have already called this->testPMConversionsIfNecessary().
SkASSERT(fDidTestPMConversions);
// The PM<->UPM tests fail or succeed together so we only need to check one.
return fDidTestPMConversions &&
GrConfigConversionEffect::kNone_PMConversion == fPMToUPMConversion;
return GrConfigConversionEffect::kNone_PMConversion == fPMToUPMConversion;
}
//////////////////////////////////////////////////////////////////////////////