[graphite] Reject creation of too-large TextureProxies
Bug: 1464023
Change-Id: I3087c81acd5947d33f000300975015813ab39340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/723816
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: James Godfrey-Kittle <jamesgk@google.com>
diff --git a/src/gpu/graphite/DrawAtlas.cpp b/src/gpu/graphite/DrawAtlas.cpp
index f0e52f9..6750967 100644
--- a/src/gpu/graphite/DrawAtlas.cpp
+++ b/src/gpu/graphite/DrawAtlas.cpp
@@ -473,13 +473,13 @@
SkASSERT(fNumActivePages < this->maxPages());
SkASSERT(!fProxies[fNumActivePages]);
- auto textureInfo = recorder->priv().caps()->getDefaultSampledTextureInfo(
- fColorType,
- /*mipmapped=*/Mipmapped::kNo,
- Protected::kNo,
- Renderable::kNo);
- fProxies[fNumActivePages].reset(
- new TextureProxy({fTextureWidth, fTextureHeight}, textureInfo, skgpu::Budgeted::kYes));
+ const Caps* caps = recorder->priv().caps();
+ auto textureInfo = caps->getDefaultSampledTextureInfo(fColorType,
+ /*mipmapped=*/Mipmapped::kNo,
+ Protected::kNo,
+ Renderable::kNo);
+ fProxies[fNumActivePages] = TextureProxy::Make(
+ caps, {fTextureWidth, fTextureHeight}, textureInfo, skgpu::Budgeted::kYes);
if (!fProxies[fNumActivePages]) {
return false;
}
diff --git a/src/gpu/graphite/ImageFactories.cpp b/src/gpu/graphite/ImageFactories.cpp
index 7f97670..24e2688 100644
--- a/src/gpu/graphite/ImageFactories.cpp
+++ b/src/gpu/graphite/ImageFactories.cpp
@@ -87,7 +87,8 @@
}
texture->setReleaseCallback(std::move(releaseHelper));
- sk_sp<TextureProxy> proxy(new TextureProxy(std::move(texture)));
+ sk_sp<TextureProxy> proxy = TextureProxy::Wrap(std::move(texture));
+ SkASSERT(proxy);
skgpu::Swizzle swizzle = caps->getReadSwizzle(ct, backendTex.info());
TextureProxyView view(std::move(proxy), swizzle);
@@ -126,7 +127,8 @@
return nullptr;
}
- sk_sp<TextureProxy> proxy = Image::MakePromiseImageLazyProxy(dimensions,
+ sk_sp<TextureProxy> proxy = Image::MakePromiseImageLazyProxy(caps,
+ dimensions,
textureInfo,
isVolatile,
fulfillProc,
@@ -180,7 +182,8 @@
// Make a lazy proxy for each plane
sk_sp<TextureProxy> proxies[4];
for (int p = 0; p < numPlanes; ++p) {
- proxies[p] = Image_YUVA::MakePromiseImageLazyProxy(planeDimensions[p],
+ proxies[p] = Image_YUVA::MakePromiseImageLazyProxy(recorder->priv().caps(),
+ planeDimensions[p],
backendTextureInfo.planeTextureInfo(p),
isVolatile,
fulfillProc,
@@ -399,7 +402,8 @@
}
texture->setReleaseCallback(releaseHelper);
- sk_sp<TextureProxy> proxy(new TextureProxy(std::move(texture)));
+ sk_sp<TextureProxy> proxy = TextureProxy::Wrap(std::move(texture));
+ SkASSERT(proxy);
textureProxyViews[plane] = TextureProxyView(std::move(proxy));
}
YUVATextureProxies yuvaProxies(recorder,
diff --git a/src/gpu/graphite/Image_Graphite.cpp b/src/gpu/graphite/Image_Graphite.cpp
index 686262d..d8a81fe 100644
--- a/src/gpu/graphite/Image_Graphite.cpp
+++ b/src/gpu/graphite/Image_Graphite.cpp
@@ -109,6 +109,7 @@
using SkImages::GraphitePromiseTextureReleaseProc;
sk_sp<TextureProxy> Image::MakePromiseImageLazyProxy(
+ const Caps* caps,
SkISize dimensions,
TextureInfo textureInfo,
Volatile isVolatile,
@@ -175,7 +176,8 @@
} callback(fulfillProc, std::move(releaseHelper), textureReleaseProc);
- return TextureProxy::MakeLazy(dimensions,
+ return TextureProxy::MakeLazy(caps,
+ dimensions,
textureInfo,
skgpu::Budgeted::kNo, // This is destined for a user's SkImage
isVolatile,
diff --git a/src/gpu/graphite/Image_Graphite.h b/src/gpu/graphite/Image_Graphite.h
index 7303052..c38c10b 100644
--- a/src/gpu/graphite/Image_Graphite.h
+++ b/src/gpu/graphite/Image_Graphite.h
@@ -38,6 +38,7 @@
TextureProxyView textureProxyView() const { return fTextureProxyView; }
static sk_sp<TextureProxy> MakePromiseImageLazyProxy(
+ const Caps*,
SkISize dimensions,
TextureInfo,
Volatile,
diff --git a/src/gpu/graphite/Image_YUVA_Graphite.cpp b/src/gpu/graphite/Image_YUVA_Graphite.cpp
index 98b5133..24491a0 100644
--- a/src/gpu/graphite/Image_YUVA_Graphite.cpp
+++ b/src/gpu/graphite/Image_YUVA_Graphite.cpp
@@ -58,6 +58,7 @@
using SkImages::GraphitePromiseTextureReleaseProc;
sk_sp<TextureProxy> Image_YUVA::MakePromiseImageLazyProxy(
+ const Caps* caps,
SkISize dimensions,
TextureInfo textureInfo,
Volatile isVolatile,
@@ -128,7 +129,8 @@
} callback(fulfillProc, std::move(releaseHelper), textureContext, textureReleaseProc);
- return TextureProxy::MakeLazy(dimensions,
+ return TextureProxy::MakeLazy(caps,
+ dimensions,
textureInfo,
skgpu::Budgeted::kNo, // This is destined for a user's SkImage
isVolatile,
diff --git a/src/gpu/graphite/Image_YUVA_Graphite.h b/src/gpu/graphite/Image_YUVA_Graphite.h
index c6c551e..4ad685f 100644
--- a/src/gpu/graphite/Image_YUVA_Graphite.h
+++ b/src/gpu/graphite/Image_YUVA_Graphite.h
@@ -45,6 +45,7 @@
}
static sk_sp<TextureProxy> MakePromiseImageLazyProxy(
+ const Caps*,
SkISize dimensions,
TextureInfo,
Volatile,
diff --git a/src/gpu/graphite/Recorder.cpp b/src/gpu/graphite/Recorder.cpp
index d71ae6f..b660c8b 100644
--- a/src/gpu/graphite/Recorder.cpp
+++ b/src/gpu/graphite/Recorder.cpp
@@ -287,7 +287,7 @@
return false;
}
- sk_sp<TextureProxy> proxy(new TextureProxy(std::move(texture)));
+ sk_sp<TextureProxy> proxy = TextureProxy::Wrap(std::move(texture));
std::vector<MipLevel> mipLevels;
mipLevels.resize(numLevels);
diff --git a/src/gpu/graphite/Surface_Graphite.cpp b/src/gpu/graphite/Surface_Graphite.cpp
index 02c0f75..94e7a7e 100644
--- a/src/gpu/graphite/Surface_Graphite.cpp
+++ b/src/gpu/graphite/Surface_Graphite.cpp
@@ -243,7 +243,7 @@
return nullptr;
}
- sk_sp<TextureProxy> proxy(new TextureProxy(std::move(texture)));
+ sk_sp<TextureProxy> proxy = TextureProxy::Wrap(std::move(texture));
sk_sp<Device> device = Device::Make(recorder,
std::move(proxy),
diff --git a/src/gpu/graphite/TextureProxy.cpp b/src/gpu/graphite/TextureProxy.cpp
index 8fb460c..3794578 100644
--- a/src/gpu/graphite/TextureProxy.cpp
+++ b/src/gpu/graphite/TextureProxy.cpp
@@ -122,32 +122,45 @@
sk_sp<TextureProxy> TextureProxy::Make(const Caps* caps,
SkISize dimensions,
+ const TextureInfo& textureInfo,
+ skgpu::Budgeted budgeted) {
+ if (dimensions.width() < 1 || dimensions.height() < 1 ||
+ dimensions.width() > caps->maxTextureSize() ||
+ dimensions.height() > caps->maxTextureSize() ||
+ !textureInfo.isValid()) {
+ return nullptr;
+ }
+
+ return sk_sp<TextureProxy>(new TextureProxy(dimensions, textureInfo, budgeted));
+}
+
+sk_sp<TextureProxy> TextureProxy::Make(const Caps* caps,
+ SkISize dimensions,
SkColorType colorType,
Mipmapped mipmapped,
Protected isProtected,
Renderable renderable,
skgpu::Budgeted budgeted) {
- if (dimensions.width() < 1 || dimensions.height() < 1) {
- return nullptr;
- }
-
TextureInfo textureInfo = caps->getDefaultSampledTextureInfo(colorType,
mipmapped,
isProtected,
renderable);
- if (!textureInfo.isValid()) {
- return nullptr;
- }
- return sk_make_sp<TextureProxy>(dimensions, textureInfo, budgeted);
+ return Make(caps, dimensions, textureInfo, budgeted);
}
-sk_sp<TextureProxy> TextureProxy::MakeLazy(SkISize dimensions,
+sk_sp<TextureProxy> TextureProxy::MakeLazy(const Caps* caps,
+ SkISize dimensions,
const TextureInfo& textureInfo,
skgpu::Budgeted budgeted,
Volatile isVolatile,
LazyInstantiateCallback&& callback) {
SkASSERT(textureInfo.isValid());
+ if (dimensions.width() < 1 || dimensions.height() < 1 ||
+ dimensions.width() > caps->maxTextureSize() ||
+ dimensions.height() > caps->maxTextureSize()) {
+ return nullptr;
+ }
return sk_sp<TextureProxy>(new TextureProxy(dimensions, textureInfo, budgeted,
isVolatile, std::move(callback)));
@@ -157,23 +170,23 @@
skgpu::Budgeted budgeted,
Volatile isVolatile,
LazyInstantiateCallback&& callback) {
- return MakeLazy(SkISize::Make(-1, -1), textureInfo, budgeted, isVolatile, std::move(callback));
+ SkASSERT(textureInfo.isValid());
+
+ return sk_sp<TextureProxy>(new TextureProxy(
+ SkISize::Make(-1, -1), textureInfo, budgeted, isVolatile, std::move(callback)));
}
sk_sp<TextureProxy> TextureProxy::MakeStorage(const Caps* caps,
SkISize dimensions,
SkColorType colorType,
skgpu::Budgeted budgeted) {
- if (dimensions.width() < 1 || dimensions.height() < 1) {
- return nullptr;
- }
-
TextureInfo textureInfo = caps->getDefaultStorageTextureInfo(colorType);
- if (!textureInfo.isValid()) {
- return nullptr;
- }
- return sk_make_sp<TextureProxy>(dimensions, textureInfo, budgeted);
+ return Make(caps, dimensions, textureInfo, budgeted);
+}
+
+sk_sp<TextureProxy> TextureProxy::Wrap(sk_sp<Texture> texture) {
+ return sk_sp<TextureProxy>(new TextureProxy(std::move(texture)));
}
#ifdef SK_DEBUG
diff --git a/src/gpu/graphite/TextureProxy.h b/src/gpu/graphite/TextureProxy.h
index 7f81cf2..7d4422a 100644
--- a/src/gpu/graphite/TextureProxy.h
+++ b/src/gpu/graphite/TextureProxy.h
@@ -25,9 +25,6 @@
class TextureProxy : public SkRefCnt {
public:
- TextureProxy(SkISize dimensions, const TextureInfo& info, skgpu::Budgeted budgeted);
- TextureProxy(sk_sp<Texture>);
-
TextureProxy() = delete;
~TextureProxy() override;
@@ -71,10 +68,15 @@
Protected,
Renderable,
skgpu::Budgeted);
+ static sk_sp<TextureProxy> Make(const Caps*,
+ SkISize dimensions,
+ const TextureInfo&,
+ skgpu::Budgeted);
using LazyInstantiateCallback = std::function<sk_sp<Texture> (ResourceProvider*)>;
- static sk_sp<TextureProxy> MakeLazy(SkISize dimensions,
+ static sk_sp<TextureProxy> MakeLazy(const Caps*,
+ SkISize dimensions,
const TextureInfo&,
skgpu::Budgeted,
Volatile,
@@ -89,12 +91,16 @@
SkColorType,
skgpu::Budgeted);
+ static sk_sp<TextureProxy> Wrap(sk_sp<Texture>);
+
private:
+ TextureProxy(SkISize dimensions, const TextureInfo& info, skgpu::Budgeted budgeted);
TextureProxy(SkISize dimensions,
const TextureInfo&,
skgpu::Budgeted,
Volatile,
LazyInstantiateCallback&&);
+ TextureProxy(sk_sp<Texture>);
#ifdef SK_DEBUG
void validateTexture(const Texture*);
diff --git a/src/gpu/graphite/TextureUtils.cpp b/src/gpu/graphite/TextureUtils.cpp
index 82db25c..65b19a8 100644
--- a/src/gpu/graphite/TextureUtils.cpp
+++ b/src/gpu/graphite/TextureUtils.cpp
@@ -101,7 +101,8 @@
}
// Create proxy
- sk_sp<TextureProxy> proxy(new TextureProxy(bmpToUpload.dimensions(), textureInfo, budgeted));
+ sk_sp<TextureProxy> proxy =
+ TextureProxy::Make(caps, bmpToUpload.dimensions(), textureInfo, budgeted);
if (!proxy) {
return {};
}
diff --git a/tests/graphite/TextureProxyTest.cpp b/tests/graphite/TextureProxyTest.cpp
index b2b6363..a6c7904 100644
--- a/tests/graphite/TextureProxyTest.cpp
+++ b/tests/graphite/TextureProxyTest.cpp
@@ -7,9 +7,14 @@
#include "tests/Test.h"
+#include "include/core/SkBitmap.h"
+#include "include/core/SkCanvas.h"
+#include "include/core/SkColorSpace.h"
#include "include/gpu/graphite/BackendTexture.h"
#include "include/gpu/graphite/Context.h"
+#include "include/gpu/graphite/Image.h"
#include "include/gpu/graphite/Recorder.h"
+#include "include/gpu/graphite/Surface.h"
#include "src/gpu/graphite/Caps.h"
#include "src/gpu/graphite/ContextPriv.h"
#include "src/gpu/graphite/RecorderPriv.h"
@@ -87,7 +92,7 @@
// Lazy, non-volatile TextureProxy, unsuccessful instantiation.
textureProxy = TextureProxy::MakeLazy(
- kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, nullCallback);
+ caps, kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, nullCallback);
REPORTER_ASSERT(reporter, textureProxy->isLazy());
REPORTER_ASSERT(reporter, !textureProxy->isFullyLazy());
REPORTER_ASSERT(reporter, !textureProxy->isVolatile());
@@ -98,7 +103,7 @@
// Lazy, non-volatile TextureProxy, successful instantiation.
textureProxy = TextureProxy::MakeLazy(
- kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, callback);
+ caps, kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, callback);
instantiateSuccess = textureProxy->lazyInstantiate(resourceProvider);
REPORTER_ASSERT(reporter, instantiateSuccess);
@@ -106,7 +111,7 @@
// Lazy, volatile TextureProxy, unsuccessful instantiation.
textureProxy = TextureProxy::MakeLazy(
- kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kYes, nullCallback);
+ caps, kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kYes, nullCallback);
REPORTER_ASSERT(reporter, textureProxy->isLazy());
REPORTER_ASSERT(reporter, !textureProxy->isFullyLazy());
REPORTER_ASSERT(reporter, textureProxy->isVolatile());
@@ -117,7 +122,7 @@
// Lazy, volatile TextureProxy, successful instantiation.
textureProxy = TextureProxy::MakeLazy(
- kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kYes, callback);
+ caps, kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kYes, callback);
instantiateSuccess = textureProxy->lazyInstantiate(resourceProvider);
REPORTER_ASSERT(reporter, instantiateSuccess);
@@ -164,9 +169,82 @@
REPORTER_ASSERT(reporter, instantiateSuccess);
textureProxy = TextureProxy::MakeLazy(
- kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, nullCallback);
+ caps, kValidSize, textureInfo, skgpu::Budgeted::kNo, Volatile::kNo, nullCallback);
instantiateSuccess = TextureProxy::InstantiateIfNotLazy(resourceProvider, textureProxy.get());
REPORTER_ASSERT(reporter, instantiateSuccess);
}
+DEF_GRAPHITE_TEST_FOR_ALL_CONTEXTS(GraphiteTextureTooLargeTest, reporter, context) {
+ std::unique_ptr<Recorder> recorder = context->makeRecorder();
+ const Caps* caps = context->priv().caps();
+
+ // Try to create a texture that is too large for the backend.
+ SkBitmap bitmap;
+ SkISize dimensions = SkISize::Make(caps->maxTextureSize() + 1, 1);
+ bitmap.allocPixels(SkImageInfo::Make(
+ dimensions, SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kPremul_SkAlphaType));
+ sk_sp<SkImage> rasterImage = SkImages::RasterFromBitmap(bitmap);
+ sk_sp<SkImage> graphiteImage =
+ SkImages::TextureFromImage(recorder.get(), rasterImage.get(), /*requiredProps=*/{});
+
+ // Image creation should have failed.
+ REPORTER_ASSERT(reporter, !graphiteImage);
+
+ // Snapping should still succeed, no texture upload should actually be attempted.
+ REPORTER_ASSERT(reporter, recorder->snap());
+}
+
+DEF_GRAPHITE_TEST_FOR_ALL_CONTEXTS(GraphiteLazyTextureInvalidDimensions, reporter, context) {
+ class FulfillContext {
+ public:
+ FulfillContext(BackendTexture backendTexture) : fBackendTexture(backendTexture) {}
+
+ static std::tuple<BackendTexture, void*> Fulfill(void* ctx) {
+ FulfillContext* self = reinterpret_cast<FulfillContext*>(ctx);
+ return {self->fBackendTexture, nullptr};
+ }
+
+ BackendTexture fBackendTexture;
+ };
+
+ std::unique_ptr<Recorder> recorder = context->makeRecorder();
+ const Caps* caps = context->priv().caps();
+
+ // Try to create textures with invalid dimensions.
+ SkISize largeDimensions = SkISize::Make(caps->maxTextureSize() + 1, 1);
+ SkISize negativeDimensions = SkISize::Make(-1, -1);
+
+ for (const SkISize& dimensions : {largeDimensions, negativeDimensions}) {
+ SkImageInfo imageInfo = SkImageInfo::Make(
+ dimensions, SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kPremul_SkAlphaType);
+ TextureInfo textureInfo = caps->getDefaultSampledTextureInfo(
+ imageInfo.colorInfo().colorType(), Mipmapped::kNo, Protected::kNo, Renderable::kNo);
+
+ // The created BackendTexture should be invalid, so an invalid texture would be used to
+ // fulfill the promise image created later, if we were to attempt to draw it.
+ BackendTexture backendTexture =
+ recorder->createBackendTexture(imageInfo.dimensions(), textureInfo);
+ FulfillContext fulfillContext(backendTexture);
+ REPORTER_ASSERT(reporter, !backendTexture.isValid());
+
+ // Drawing should still succeed, as no image draw should actually be attempted with this
+ // texture.
+ SkImageInfo surfaceImageInfo = SkImageInfo::Make(
+ 1, 1, SkColorType::kRGBA_8888_SkColorType, SkAlphaType::kPremul_SkAlphaType);
+ sk_sp<SkSurface> surface = SkSurfaces::RenderTarget(recorder.get(), surfaceImageInfo);
+ sk_sp<SkImage> promiseImage = SkImages::PromiseTextureFrom(recorder.get(),
+ imageInfo.dimensions(),
+ textureInfo,
+ imageInfo.colorInfo(),
+ Volatile::kNo,
+ FulfillContext::Fulfill,
+ nullptr,
+ nullptr,
+ &fulfillContext);
+ surface->getCanvas()->drawImage(promiseImage, 0.0f, 0.0f);
+ std::unique_ptr<Recording> recording = recorder->snap();
+ REPORTER_ASSERT(reporter, context->insertRecording({recording.get()}));
+ }
+}
+
} // namespace skgpu::graphite
