Skip to content

Commit cfdc5b6

Browse files
committed
Move uses of SkSurface::makeImageSnapshot to makeTemporaryImage.
In graphite, makeImageSnapshot no longer has a copy on write symantic. Instead it will always make a copy which is not need in many cases. This CL changes the the uses of makeImageSnapshot where a copy isn't needed to call makeTemporaryImage instead. This avoids the extra copies in Graphite. Besides possible performance wins, this also reduces a lot of extra memory that was being allocated for these copies. This is most seen in situtations using a lot of blurs. Bug: b/293371537 , b/385380555 Test: manual comparison of memory metric Flag: com.android.graphics.surfaceflinger.flags.graphite_renderengine Change-Id: Ia9ed5c778f083b7a506158510438e656a4625a67
1 parent 04a3b4f commit cfdc5b6

6 files changed

Lines changed: 33 additions & 18 deletions

File tree

libs/renderengine/skia/SkiaRenderEngine.cpp

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -838,8 +838,7 @@ void SkiaRenderEngine::drawLayersInternal(
838838
LOG_ALWAYS_FATAL_IF(activeSurface == dstSurface);
839839
LOG_ALWAYS_FATAL_IF(canvas == dstCanvas);
840840

841-
// save a snapshot of the activeSurface to use as input to the blur shaders
842-
blurInput = activeSurface->makeImageSnapshot();
841+
blurInput = activeSurface->makeTemporaryImage();
843842

844843
// blit the offscreen framebuffer into the destination AHB. This ensures that
845844
// even if the blurred image does not cover the screen (for example, during
@@ -853,12 +852,9 @@ void SkiaRenderEngine::drawLayersInternal(
853852
dstCanvas->drawAnnotation(SkRect::Make(dstCanvas->imageInfo().dimensions()),
854853
String8::format("SurfaceID|%" PRId64, id).c_str(),
855854
nullptr);
856-
dstCanvas->drawImage(blurInput, 0, 0, SkSamplingOptions(), &paint);
857-
} else {
858-
activeSurface->draw(dstCanvas, 0, 0, SkSamplingOptions(), &paint);
859855
}
856+
dstCanvas->drawImage(blurInput, 0, 0, SkSamplingOptions(), &paint);
860857
}
861-
862858
// assign dstCanvas to canvas and ensure that the canvas state is up to date
863859
canvas = dstCanvas;
864860
surfaceAutoSaveRestore.replace(canvas);
@@ -891,12 +887,6 @@ void SkiaRenderEngine::drawLayersInternal(
891887
if (mBlurFilter && layerHasBlur(layer, ctModifiesAlpha)) {
892888
std::unordered_map<uint32_t, sk_sp<SkImage>> cachedBlurs;
893889

894-
// if multiple layers have blur, then we need to take a snapshot now because
895-
// only the lowest layer will have blurImage populated earlier
896-
if (!blurInput) {
897-
blurInput = activeSurface->makeImageSnapshot();
898-
}
899-
900890
// rect to be blurred in the coordinate space of blurInput
901891
SkRect blurRect = canvas->getTotalMatrix().mapRect(bounds.rect());
902892

@@ -920,6 +910,29 @@ void SkiaRenderEngine::drawLayersInternal(
920910

921911
// TODO(b/182216890): Filter out empty layers earlier
922912
if (blurRect.width() > 0 && blurRect.height() > 0) {
913+
// if multiple layers have blur, then we need to take a snapshot now because
914+
// only the lowest layer will have blurImage populated earlier
915+
if (!blurInput) {
916+
bool requiresCrossFadeWithBlurInput = false;
917+
if (layer.backgroundBlurRadius > 0 &&
918+
layer.backgroundBlurRadius < mBlurFilter->getMaxCrossFadeRadius()) {
919+
requiresCrossFadeWithBlurInput = true;
920+
}
921+
for (auto region : layer.blurRegions) {
922+
if (region.blurRadius < mBlurFilter->getMaxCrossFadeRadius()) {
923+
requiresCrossFadeWithBlurInput = true;
924+
}
925+
}
926+
if (requiresCrossFadeWithBlurInput) {
927+
// If we require cross fading with the blur input, we need to make sure we
928+
// make a copy of the surface to the image since we will be writing to the
929+
// surface while sampling the blurInput.
930+
blurInput = activeSurface->makeImageSnapshot();
931+
} else {
932+
blurInput = activeSurface->makeTemporaryImage();
933+
}
934+
}
935+
923936
if (layer.backgroundBlurRadius > 0) {
924937
SFTRACE_NAME("BackgroundBlur");
925938
auto blurredImage = mBlurFilter->generate(context, layer.backgroundBlurRadius,

libs/renderengine/skia/filters/BlurFilter.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ void BlurFilter::drawBlurRegion(SkCanvas* canvas, const SkRRect& effectRegion,
9090
linearSampling, &blurMatrix);
9191

9292
if (blurRadius < mMaxCrossFadeRadius) {
93+
LOG_ALWAYS_FATAL_IF(!input);
94+
9395
// For sampling Skia's API expects the inverse of what logically seems appropriate. In this
9496
// case you might expect the matrix to simply be the canvas matrix.
9597
SkMatrix inputMatrix;

libs/renderengine/skia/filters/GaussianBlurFilter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ sk_sp<SkImage> GaussianBlurFilter::generate(SkiaGpuContext* context, const uint3
6464
SkSamplingOptions{SkFilterMode::kLinear, SkMipmapMode::kNone},
6565
&paint,
6666
SkCanvas::SrcRectConstraint::kFast_SrcRectConstraint);
67-
return surface->makeImageSnapshot();
67+
return surface->makeTemporaryImage();
6868
}
6969

7070
} // namespace skia

libs/renderengine/skia/filters/KawaseBlurDualFilter.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,14 @@ sk_sp<SkImage> KawaseBlurDualFilter::generate(SkiaGpuContext* context, const uin
167167
}
168168
// Next the remaining downscale blur passes.
169169
for (int i = 0; i < filterPasses; i++) {
170-
blurInto(surfaces[i + 1], surfaces[i]->makeImageSnapshot(), kWeights[1 + i] * step, 1.0f);
170+
blurInto(surfaces[i + 1], surfaces[i]->makeTemporaryImage(), kWeights[1 + i] * step, 1.0f);
171171
}
172172
// Finally blur+upscale back to our original size.
173173
for (int i = filterPasses - 1; i >= 0; i--) {
174-
blurInto(surfaces[i], surfaces[i + 1]->makeImageSnapshot(), kWeights[4 - i] * step,
174+
blurInto(surfaces[i], surfaces[i + 1]->makeTemporaryImage(), kWeights[4 - i] * step,
175175
std::min(1.0f, filterDepth - i));
176176
}
177-
return surfaces[0]->makeImageSnapshot();
177+
return surfaces[0]->makeTemporaryImage();
178178
}
179179

180180
} // namespace skia

libs/renderengine/skia/filters/KawaseBlurFilter.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ static sk_sp<SkImage> makeImage(SkSurface* surface, SkRuntimeShaderBuilder* buil
7070
paint.setShader(std::move(shader));
7171
paint.setBlendMode(SkBlendMode::kSrc);
7272
surface->getCanvas()->drawPaint(paint);
73-
return surface->makeImageSnapshot();
73+
return surface->makeTemporaryImage();
7474
}
7575

7676
sk_sp<SkImage> KawaseBlurFilter::generate(SkiaGpuContext* context, const uint32_t blurRadius,

libs/renderengine/skia/filters/MouriMap.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ sk_sp<SkImage> makeImage(SkSurface* surface, const SkRuntimeShaderBuilder& build
104104
paint.setShader(std::move(shader));
105105
paint.setBlendMode(SkBlendMode::kSrc);
106106
surface->getCanvas()->drawPaint(paint);
107-
return surface->makeImageSnapshot();
107+
return surface->makeTemporaryImage();
108108
}
109109

110110
} // namespace

0 commit comments

Comments
 (0)