From 98fa4f9e7b33a3004ce9142c9acd4300391b9a0e Mon Sep 17 00:00:00 2001 From: sergeyv Date: Mon, 24 Oct 2016 15:35:21 -0700 Subject: [PATCH 1/2] Use Bitmap in Texture.upload Test: refactoring cl. bug:32216791 Change-Id: Ib0b16c878d8371e0471e9a502f55626ec5999c60 --- libs/hwui/PathCache.cpp | 40 +++++++++++++--------------- libs/hwui/PathCache.h | 50 ++++++++++++++++------------------- libs/hwui/RecordingCanvas.cpp | 1 - libs/hwui/Texture.cpp | 26 +++++++++--------- libs/hwui/Texture.h | 8 +++--- libs/hwui/TextureCache.cpp | 12 +++------ libs/hwui/hwui/Bitmap.h | 8 ++++++ 7 files changed, 68 insertions(+), 77 deletions(-) diff --git a/libs/hwui/PathCache.cpp b/libs/hwui/PathCache.cpp index d46c46f9381f3..cc96de71df826 100644 --- a/libs/hwui/PathCache.cpp +++ b/libs/hwui/PathCache.cpp @@ -138,11 +138,6 @@ static void computePathBounds(const SkPath* path, const SkPaint* paint, PathText height = uint32_t(pathHeight + texture->offset * 2.0 + 0.5); } -static void initBitmap(SkBitmap& bitmap, uint32_t width, uint32_t height) { - bitmap.allocPixels(SkImageInfo::MakeA8(width, height)); - bitmap.eraseColor(0); -} - static void initPaint(SkPaint& paint) { // Make sure the paint is opaque, color, alpha, filter, etc. // will be applied later when compositing the alpha8 texture @@ -154,7 +149,7 @@ static void initPaint(SkPaint& paint) { paint.setBlendMode(SkBlendMode::kSrc); } -static SkBitmap* drawPath(const SkPath* path, const SkPaint* paint, PathTexture* texture, +static sk_sp drawPath(const SkPath* path, const SkPaint* paint, PathTexture* texture, uint32_t maxTextureSize) { uint32_t width, height; computePathBounds(path, paint, texture, width, height); @@ -164,13 +159,14 @@ static SkBitmap* drawPath(const SkPath* path, const SkPaint* paint, PathTexture* return nullptr; } - SkBitmap* bitmap = new SkBitmap(); - initBitmap(*bitmap, width, height); - + sk_sp bitmap = Bitmap::allocateHeapBitmap(SkImageInfo::MakeA8(width, height)); SkPaint pathPaint(*paint); initPaint(pathPaint); - SkCanvas canvas(*bitmap); + SkBitmap skBitmap; + bitmap->getSkBitmap(&skBitmap); + skBitmap.eraseColor(0); + SkCanvas canvas(skBitmap); canvas.translate(-texture->left + texture->offset, -texture->top + texture->offset); canvas.drawPath(*path, pathPaint); return bitmap; @@ -227,7 +223,7 @@ void PathCache::removeTexture(PathTexture* texture) { // If there is a pending task we must wait for it to return // before attempting our cleanup - const sp >& task = texture->task(); + const sp& task = texture->task(); if (task != nullptr) { task->getResult(); texture->clearTask(); @@ -280,20 +276,20 @@ PathTexture* PathCache::addTexture(const PathDescription& entry, const SkPath *p ATRACE_NAME("Generate Path Texture"); PathTexture* texture = new PathTexture(Caches::getInstance(), path->getGenerationID()); - std::unique_ptr bitmap(drawPath(path, paint, texture, mMaxTextureSize)); - if (!bitmap.get()) { + sk_sp bitmap(drawPath(path, paint, texture, mMaxTextureSize)); + if (!bitmap) { delete texture; return nullptr; } purgeCache(bitmap->width(), bitmap->height()); - generateTexture(entry, bitmap.get(), texture); + generateTexture(entry, *bitmap, texture); return texture; } -void PathCache::generateTexture(const PathDescription& entry, SkBitmap* bitmap, +void PathCache::generateTexture(const PathDescription& entry, Bitmap& bitmap, PathTexture* texture, bool addToCache) { - generateTexture(*bitmap, texture); + generateTexture(bitmap, texture); // Note here that we upload to a texture even if it's bigger than mMaxSize. // Such an entry in mCache will only be temporary, since it will be evicted @@ -314,7 +310,7 @@ void PathCache::clear() { mCache.clear(); } -void PathCache::generateTexture(SkBitmap& bitmap, Texture* texture) { +void PathCache::generateTexture(Bitmap& bitmap, Texture* texture) { ATRACE_NAME("Upload Path Texture"); texture->upload(bitmap); texture->setFilter(GL_LINEAR); @@ -325,10 +321,10 @@ void PathCache::generateTexture(SkBitmap& bitmap, Texture* texture) { /////////////////////////////////////////////////////////////////////////////// PathCache::PathProcessor::PathProcessor(Caches& caches): - TaskProcessor(&caches.tasks), mMaxTextureSize(caches.maxTextureSize) { + TaskProcessor >(&caches.tasks), mMaxTextureSize(caches.maxTextureSize) { } -void PathCache::PathProcessor::onProcess(const sp >& task) { +void PathCache::PathProcessor::onProcess(const sp > >& task) { PathTask* t = static_cast(task.get()); ATRACE_NAME("pathPrecache"); @@ -377,13 +373,13 @@ PathTexture* PathCache::get(const SkPath* path, const SkPaint* paint) { } else { // A bitmap is attached to the texture, this means we need to // upload it as a GL texture - const sp >& task = texture->task(); + const sp& task = texture->task(); if (task != nullptr) { // But we must first wait for the worker thread to be done // producing the bitmap, so let's wait - SkBitmap* bitmap = task->getResult(); + sk_sp bitmap = task->getResult(); if (bitmap) { - generateTexture(entry, bitmap, texture, false); + generateTexture(entry, *bitmap, texture, false); texture->clearTask(); } else { texture->clearTask(); diff --git a/libs/hwui/PathCache.h b/libs/hwui/PathCache.h index 18bcc566296fd..7bd190df951b5 100644 --- a/libs/hwui/PathCache.h +++ b/libs/hwui/PathCache.h @@ -19,6 +19,7 @@ #include "Debug.h" #include "Texture.h" +#include "hwui/Bitmap.h" #include "thread/Task.h" #include "thread/TaskProcessor.h" #include "utils/Macros.h" @@ -32,7 +33,6 @@ #include -class SkBitmap; class SkCanvas; class SkPaint; struct SkRect; @@ -41,7 +41,6 @@ namespace android { namespace uirenderer { class Caches; - /////////////////////////////////////////////////////////////////////////////// // Defines /////////////////////////////////////////////////////////////////////////////// @@ -57,6 +56,21 @@ class Caches; // Classes /////////////////////////////////////////////////////////////////////////////// +struct PathTexture; +class PathTask: public Task> { +public: + PathTask(const SkPath* path, const SkPaint* paint, PathTexture* texture): + path(*path), paint(*paint), texture(texture) { + } + + // copied, since input path not guaranteed to survive for duration of task + const SkPath path; + + // copied, since input paint may not be immutable + const SkPaint paint; + PathTexture* texture; +}; + /** * Alpha texture used to represent a path. */ @@ -83,11 +97,11 @@ struct PathTexture: public Texture { */ float offset = 0; - sp > task() const { + sp task() const { return mTask; } - void setTask(const sp >& task) { + void setTask(const sp& task) { mTask = task; } @@ -98,7 +112,7 @@ struct PathTexture: public Texture { } private: - sp > mTask; + sp mTask; }; // struct PathTexture enum class ShapeType { @@ -222,13 +236,12 @@ public: private: PathTexture* addTexture(const PathDescription& entry, const SkPath *path, const SkPaint* paint); - PathTexture* addTexture(const PathDescription& entry, SkBitmap* bitmap); /** * Generates the texture from a bitmap into the specified texture structure. */ - void generateTexture(SkBitmap& bitmap, Texture* texture); - void generateTexture(const PathDescription& entry, SkBitmap* bitmap, PathTexture* texture, + void generateTexture(Bitmap& bitmap, Texture* texture); + void generateTexture(const PathDescription& entry, Bitmap& bitmap, PathTexture* texture, bool addToCache = true); PathTexture* get(const PathDescription& entry) { @@ -245,30 +258,13 @@ private: void init(); - class PathTask: public Task { - public: - PathTask(const SkPath* path, const SkPaint* paint, PathTexture* texture): - path(*path), paint(*paint), texture(texture) { - } - ~PathTask() { - delete future()->get(); - } - - // copied, since input path not guaranteed to survive for duration of task - const SkPath path; - - // copied, since input paint may not be immutable - const SkPaint paint; - PathTexture* texture; - }; - - class PathProcessor: public TaskProcessor { + class PathProcessor: public TaskProcessor > { public: explicit PathProcessor(Caches& caches); ~PathProcessor() { } - virtual void onProcess(const sp >& task) override; + virtual void onProcess(const sp > >& task) override; private: uint32_t mMaxTextureSize; diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp index f5bcba284e2b0..9d18484f39f04 100644 --- a/libs/hwui/RecordingCanvas.cpp +++ b/libs/hwui/RecordingCanvas.cpp @@ -666,7 +666,6 @@ void RecordingCanvas::refBitmapsInShader(const SkShader* shader) { SkBitmap bitmap; SkShader::TileMode xy[2]; if (shader->isABitmap(&bitmap, nullptr, xy)) { - // TODO: create hwui-owned BitmapShader. Bitmap* hwuiBitmap = static_cast(bitmap.pixelRef()); refBitmap(*hwuiBitmap); return; diff --git a/libs/hwui/Texture.cpp b/libs/hwui/Texture.cpp index 908f572659064..09e107eae1329 100644 --- a/libs/hwui/Texture.cpp +++ b/libs/hwui/Texture.cpp @@ -171,12 +171,6 @@ static void uploadToTexture(bool resize, GLint internalFormat, GLenum format, GL } } -static void uploadSkBitmapToTexture(const SkBitmap& bitmap, - bool resize, GLint internalFormat, GLenum format, GLenum type) { - uploadToTexture(resize, internalFormat, format, type, bitmap.rowBytesAsPixels(), - bitmap.bytesPerPixel(), bitmap.width(), bitmap.height(), bitmap.getPixels()); -} - static void colorTypeToGlFormatAndType(const Caches& caches, SkColorType colorType, bool needSRGB, GLint* outInternalFormat, GLint* outFormat, GLint* outType) { switch (colorType) { @@ -218,9 +212,7 @@ static void colorTypeToGlFormatAndType(const Caches& caches, SkColorType colorTy } } -void Texture::upload(const SkBitmap& bitmap) { - SkAutoLockPixels alp(bitmap); - +void Texture::upload(Bitmap& bitmap) { if (!bitmap.readyToDraw()) { ALOGE("Cannot generate texture from bitmap"); return; @@ -244,7 +236,7 @@ void Texture::upload(const SkBitmap& bitmap) { } sk_sp sRGB = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named); - bool needSRGB = bitmap.colorSpace() == sRGB.get(); + bool needSRGB = bitmap.info().colorSpace() == sRGB.get(); GLint internalFormat, format, type; colorTypeToGlFormatAndType(mCaches, bitmap.colorType(), needSRGB, &internalFormat, &format, &type); @@ -264,15 +256,21 @@ void Texture::upload(const SkBitmap& bitmap) { SkBitmap rgbaBitmap; rgbaBitmap.allocPixels(SkImageInfo::MakeN32( - mWidth, mHeight, bitmap.alphaType(), hasSRGB ? sRGB : nullptr)); + mWidth, mHeight, bitmap.info().alphaType(), hasSRGB ? sRGB : nullptr)); rgbaBitmap.eraseColor(0); SkCanvas canvas(rgbaBitmap); - canvas.drawBitmap(bitmap, 0.0f, 0.0f, nullptr); + SkBitmap skBitmap; + bitmap.getSkBitmap(&skBitmap); + canvas.drawBitmap(skBitmap, 0.0f, 0.0f, nullptr); + + uploadToTexture(needsAlloc, internalFormat, format, type, rgbaBitmap.rowBytesAsPixels(), + rgbaBitmap.bytesPerPixel(), rgbaBitmap.width(), + rgbaBitmap.height(), rgbaBitmap.getPixels()); - uploadSkBitmapToTexture(rgbaBitmap, needsAlloc, internalFormat, format, type); } else { - uploadSkBitmapToTexture(bitmap, needsAlloc, internalFormat, format, type); + uploadToTexture(needsAlloc, internalFormat, format, type, bitmap.rowBytesAsPixels(), + bitmap.info().bytesPerPixel(), bitmap.width(), bitmap.height(), bitmap.pixels()); } if (canMipMap) { diff --git a/libs/hwui/Texture.h b/libs/hwui/Texture.h index aa8a6d3fae824..57cfc2b0a9553 100644 --- a/libs/hwui/Texture.h +++ b/libs/hwui/Texture.h @@ -18,9 +18,9 @@ #define ANDROID_HWUI_TEXTURE_H #include "GpuMemoryTracker.h" +#include "hwui/Bitmap.h" #include -#include namespace android { namespace uirenderer { @@ -74,13 +74,13 @@ public: } /** - * Updates this Texture with the contents of the provided SkBitmap, + * Updates this Texture with the contents of the provided Bitmap, * also setting the appropriate width, height, and format. It is not necessary * to call resize() prior to this. * - * Note this does not set the generation from the SkBitmap. + * Note this does not set the generation from the Bitmap. */ - void upload(const SkBitmap& source); + void upload(Bitmap& source); /** * Basically glTexImage2D/glTexSubImage2D. diff --git a/libs/hwui/TextureCache.cpp b/libs/hwui/TextureCache.cpp index 08641b7234dd7..14ffc85a4f687 100644 --- a/libs/hwui/TextureCache.cpp +++ b/libs/hwui/TextureCache.cpp @@ -127,9 +127,7 @@ Texture* TextureCache::getCachedTexture(Bitmap* bitmap) { texture = new Texture(Caches::getInstance()); texture->bitmapSize = size; texture->generation = bitmap->getGenerationID(); - SkBitmap skBitmap; - bitmap->getSkBitmap(&skBitmap); - texture->upload(skBitmap); + texture->upload(*bitmap); mSize += size; TEXTURE_LOGD("TextureCache::get: create texture(%p): name, size, mSize = %d, %d, %d", @@ -142,9 +140,7 @@ Texture* TextureCache::getCachedTexture(Bitmap* bitmap) { } else if (!texture->isInUse && bitmap->getGenerationID() != texture->generation) { // Texture was in the cache but is dirty, re-upload // TODO: Re-adjust the cache size if the bitmap's dimensions have changed - SkBitmap skBitmap; - bitmap->getSkBitmap(&skBitmap); - texture->upload(skBitmap); + texture->upload(*bitmap); texture->generation = bitmap->getGenerationID(); } @@ -174,9 +170,7 @@ Texture* TextureCache::get(Bitmap* bitmap) { const uint32_t size = bitmap->rowBytes() * bitmap->height(); texture = new Texture(Caches::getInstance()); texture->bitmapSize = size; - SkBitmap skBitmap; - bitmap->getSkBitmap(&skBitmap); - texture->upload(skBitmap); + texture->upload(*bitmap); texture->generation = bitmap->getGenerationID(); texture->cleanup = true; } diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h index e86ac11673c50..c175690265761 100644 --- a/libs/hwui/hwui/Bitmap.h +++ b/libs/hwui/hwui/Bitmap.h @@ -57,6 +57,11 @@ public: // doing on a Bitmap type, not a SkPixelRef, so static // dispatching will do what we want. size_t rowBytes() const { return mRowBytes; } + + int rowBytesAsPixels() const { + return mRowBytes >> info().shiftPerPixel(); + } + void reconfigure(const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable); void reconfigure(const SkImageInfo& info); void setAlphaType(SkAlphaType alphaType); @@ -73,6 +78,9 @@ public: SkColorType colorType() const { return info().colorType(); } void getBounds(SkRect* bounds) const; + bool readyToDraw() const { + return this->colorType() != kIndex_8_SkColorType || mColorTable; + } protected: virtual bool onNewLockPixels(LockRec* rec) override; virtual void onUnlockPixels() override { }; From 2a38c42e921451abebb4ee5f5ecd738f1b6b04ed Mon Sep 17 00:00:00 2001 From: sergeyv Date: Tue, 25 Oct 2016 15:21:50 -0700 Subject: [PATCH 2/2] Add target to texture Test: refactoring cl. bug:32413624 Change-Id: I94b1c31cd4e0712dfcfd7777a0012424c1bf0dca --- libs/hwui/Glop.h | 1 - libs/hwui/GlopBuilder.cpp | 29 ++++++++--------- libs/hwui/GlopBuilder.h | 5 ++- libs/hwui/Layer.cpp | 3 +- libs/hwui/Layer.h | 18 +++++------ libs/hwui/Readback.cpp | 4 +-- libs/hwui/Texture.cpp | 38 +++++++++++++---------- libs/hwui/Texture.h | 25 +++++++++------ libs/hwui/renderstate/RenderState.cpp | 6 ++-- libs/hwui/tests/unit/GlopBuilderTests.cpp | 8 +++-- 10 files changed, 70 insertions(+), 67 deletions(-) diff --git a/libs/hwui/Glop.h b/libs/hwui/Glop.h index 46dd598711b86..b396e22431f64 100644 --- a/libs/hwui/Glop.h +++ b/libs/hwui/Glop.h @@ -119,7 +119,6 @@ public: struct TextureData { Texture* texture; - GLenum target; GLenum filter; GLenum clamp; Matrix4* textureTransform; diff --git a/libs/hwui/GlopBuilder.cpp b/libs/hwui/GlopBuilder.cpp index f14b50a843122..3b5fc718a0365 100644 --- a/libs/hwui/GlopBuilder.cpp +++ b/libs/hwui/GlopBuilder.cpp @@ -308,8 +308,7 @@ GlopBuilder& GlopBuilder::setFillTexturePaint(Texture& texture, GLenum filter = (textureFillFlags & TextureFillFlags::ForceFilter) ? GL_LINEAR : PaintUtils::getFilter(paint); - mOutGlop->fill.texture = { &texture, - GL_TEXTURE_2D, filter, GL_CLAMP_TO_EDGE, nullptr }; + mOutGlop->fill.texture = { &texture, filter, GL_CLAMP_TO_EDGE, nullptr }; if (paint) { int color = paint->getColor(); @@ -352,10 +351,10 @@ GlopBuilder& GlopBuilder::setFillPaint(const SkPaint& paint, float alphaScale, b if (CC_LIKELY(!shadowInterp)) { mOutGlop->fill.texture = { - nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; } else { mOutGlop->fill.texture = { - mCaches.textureState().getShadowLutTexture(), GL_TEXTURE_2D, + mCaches.textureState().getShadowLutTexture(), GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; } @@ -373,7 +372,7 @@ GlopBuilder& GlopBuilder::setFillPathTexturePaint(PathTexture& texture, REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); //specify invalid filter/clamp, since these are always static for PathTextures - mOutGlop->fill.texture = { &texture, GL_TEXTURE_2D, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + mOutGlop->fill.texture = { &texture, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; setFill(paint.getColor(), alphaScale, paint.getBlendMode(), Blend::ModeOrderSwap::NoSwap, @@ -390,7 +389,7 @@ GlopBuilder& GlopBuilder::setFillShadowTexturePaint(ShadowTexture& texture, int REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); //specify invalid filter/clamp, since these are always static for ShadowTextures - mOutGlop->fill.texture = { &texture, GL_TEXTURE_2D, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + mOutGlop->fill.texture = { &texture, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; const int ALPHA_BITMASK = SK_ColorBLACK; const int COLOR_BITMASK = ~ALPHA_BITMASK; @@ -412,7 +411,7 @@ GlopBuilder& GlopBuilder::setFillBlack() { TRIGGER_STAGE(kFillStage); REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); - mOutGlop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + mOutGlop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; setFill(SK_ColorBLACK, 1.0f, SkBlendMode::kSrcOver, Blend::ModeOrderSwap::NoSwap, nullptr, nullptr); return *this; @@ -422,7 +421,7 @@ GlopBuilder& GlopBuilder::setFillClear() { TRIGGER_STAGE(kFillStage); REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); - mOutGlop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + mOutGlop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; setFill(SK_ColorBLACK, 1.0f, SkBlendMode::kClear, Blend::ModeOrderSwap::NoSwap, nullptr, nullptr); return *this; @@ -433,8 +432,7 @@ GlopBuilder& GlopBuilder::setFillLayer(Texture& texture, const SkColorFilter* co TRIGGER_STAGE(kFillStage); REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); - mOutGlop->fill.texture = { &texture, - GL_TEXTURE_2D, GL_LINEAR, GL_CLAMP_TO_EDGE, nullptr }; + mOutGlop->fill.texture = { &texture, GL_LINEAR, GL_CLAMP_TO_EDGE, nullptr }; setFill(SK_ColorWHITE, alpha, mode, modeUsage, nullptr, colorFilter); @@ -447,7 +445,7 @@ GlopBuilder& GlopBuilder::setFillTextureLayer(Layer& layer, float alpha) { REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); mOutGlop->fill.texture = { &(layer.getTexture()), - layer.getRenderTarget(), GL_LINEAR, GL_CLAMP_TO_EDGE, &layer.getTexTransform() }; + GL_LINEAR, GL_CLAMP_TO_EDGE, &layer.getTexTransform() }; setFill(SK_ColorWHITE, alpha, layer.getMode(), Blend::ModeOrderSwap::NoSwap, nullptr, layer.getColorFilter()); @@ -461,9 +459,7 @@ GlopBuilder& GlopBuilder::setFillExternalTexture(Texture& texture, Matrix4& text TRIGGER_STAGE(kFillStage); REQUIRE_STAGES(kMeshStage | kRoundRectClipStage); - mOutGlop->fill.texture = { &texture, - GL_TEXTURE_EXTERNAL_OES, GL_LINEAR, GL_CLAMP_TO_EDGE, - &textureTransform }; + mOutGlop->fill.texture = { &texture, GL_LINEAR, GL_CLAMP_TO_EDGE, &textureTransform }; setFill(SK_ColorWHITE, 1.0f, SkBlendMode::kSrc, Blend::ModeOrderSwap::NoSwap, nullptr, nullptr); @@ -603,7 +599,7 @@ void verify(const ProgramDescription& description, const Glop& glop) { void GlopBuilder::build() { REQUIRE_STAGES(kAllStages); if (mOutGlop->mesh.vertices.attribFlags & VertexAttribFlags::TextureCoord) { - if (mOutGlop->fill.texture.target == GL_TEXTURE_2D) { + if (mOutGlop->fill.texture.texture->target() == GL_TEXTURE_2D) { mDescription.hasTexture = true; } else { mDescription.hasExternalTexture = true; @@ -668,7 +664,8 @@ void GlopBuilder::dump(const Glop& glop) { ALOGD(" program %p", fill.program); if (fill.texture.texture) { ALOGD(" texture %p, target %d, filter %d, clamp %d", - fill.texture.texture, fill.texture.target, fill.texture.filter, fill.texture.clamp); + fill.texture.texture, fill.texture.texture->target(), + fill.texture.filter, fill.texture.clamp); if (fill.texture.textureTransform) { fill.texture.textureTransform->dump("texture transform"); } diff --git a/libs/hwui/GlopBuilder.h b/libs/hwui/GlopBuilder.h index d511ccbcb71a3..8a8b652c393ce 100644 --- a/libs/hwui/GlopBuilder.h +++ b/libs/hwui/GlopBuilder.h @@ -72,9 +72,8 @@ public: GlopBuilder& setFillLayer(Texture& texture, const SkColorFilter* colorFilter, float alpha, SkBlendMode mode, Blend::ModeOrderSwap modeUsage); GlopBuilder& setFillTextureLayer(Layer& layer, float alpha); - // TODO: Texture should probably know and own its target. - // setFillLayer() forces it to GL_TEXTURE which isn't always correct. - // Similarly setFillLayer normally forces its own wrap & filter mode + // TODO: setFillLayer normally forces its own wrap & filter mode, + // which isn't always correct. GlopBuilder& setFillExternalTexture(Texture& texture, Matrix4& textureTransform); GlopBuilder& setTransform(const Matrix4& canvas, const int transformFlags); diff --git a/libs/hwui/Layer.cpp b/libs/hwui/Layer.cpp index 4e12bcef8a4fe..88817efa80acb 100644 --- a/libs/hwui/Layer.cpp +++ b/libs/hwui/Layer.cpp @@ -41,7 +41,6 @@ Layer::Layer(RenderState& renderState, uint32_t layerWidth, uint32_t layerHeight // TODO: This is a violation of Android's typical ref counting, but it // preserves the old inc/dec ref locations. This should be changed... incStrong(nullptr); - renderTarget = GL_NONE; // see DeferredLayerUpdater::updateLayer() texture.mWidth = layerWidth; texture.mHeight = layerHeight; renderState.registerLayer(this); @@ -66,7 +65,7 @@ void Layer::setColorFilter(SkColorFilter* filter) { void Layer::bindTexture() const { if (texture.mId) { - caches.textureState().bindTexture(renderTarget, texture.mId); + caches.textureState().bindTexture(texture.target(), texture.mId); } } diff --git a/libs/hwui/Layer.h b/libs/hwui/Layer.h index 9874ce29cb3c2..8e71cd11599d3 100644 --- a/libs/hwui/Layer.h +++ b/libs/hwui/Layer.h @@ -75,7 +75,8 @@ public: } void setSize(uint32_t width, uint32_t height) { - texture.updateSize(width, height, texture.internalFormat(), texture.format()); + texture.updateSize(width, height, texture.internalFormat(), texture.format(), + texture.target()); } inline void setBlend(bool blend) { @@ -120,23 +121,23 @@ public: } inline GLenum getRenderTarget() const { - return renderTarget; + return texture.target(); } inline void setRenderTarget(GLenum renderTarget) { - this->renderTarget = renderTarget; + texture.mTarget = renderTarget; } inline bool isRenderable() const { - return renderTarget != GL_NONE; + return texture.target() != GL_NONE; } void setWrap(GLenum wrap, bool bindTexture = false, bool force = false) { - texture.setWrap(wrap, bindTexture, force, renderTarget); + texture.setWrap(wrap, bindTexture, force); } void setFilter(GLenum filter, bool bindTexture = false, bool force = false) { - texture.setFilter(filter, bindTexture, force, renderTarget); + texture.setFilter(filter, bindTexture, force); } inline SkColorFilter* getColorFilter() const { @@ -185,11 +186,6 @@ private: */ Texture texture; - /** - * Indicates the render target. - */ - GLenum renderTarget = GL_TEXTURE_2D; - /** * Color filter used to draw this layer. Optional. */ diff --git a/libs/hwui/Readback.cpp b/libs/hwui/Readback.cpp index 22c6dfc6b55ac..1645218495eb5 100644 --- a/libs/hwui/Readback.cpp +++ b/libs/hwui/Readback.cpp @@ -196,8 +196,8 @@ CopyResult Readback::copySurfaceInto(renderthread::RenderThread& renderThread, } Texture sourceTexture(caches); - sourceTexture.wrap(sourceTexId, - sourceBuffer->getWidth(), sourceBuffer->getHeight(), 0, 0 /* total lie */); + sourceTexture.wrap(sourceTexId, sourceBuffer->getWidth(), + sourceBuffer->getHeight(), 0, 0 /* total lie */, GL_TEXTURE_EXTERNAL_OES); CopyResult copyResult = copyTextureInto(caches, renderThread.renderState(), sourceTexture, texTransform, srcRect, bitmap); diff --git a/libs/hwui/Texture.cpp b/libs/hwui/Texture.cpp index 09e107eae1329..c4a65f6fd5456 100644 --- a/libs/hwui/Texture.cpp +++ b/libs/hwui/Texture.cpp @@ -48,37 +48,34 @@ static int bytesPerPixel(GLint glFormat) { } } -void Texture::setWrapST(GLenum wrapS, GLenum wrapT, bool bindTexture, bool force, - GLenum renderTarget) { +void Texture::setWrapST(GLenum wrapS, GLenum wrapT, bool bindTexture, bool force) { if (force || wrapS != mWrapS || wrapT != mWrapT) { mWrapS = wrapS; mWrapT = wrapT; if (bindTexture) { - mCaches.textureState().bindTexture(renderTarget, mId); + mCaches.textureState().bindTexture(mTarget, mId); } - glTexParameteri(renderTarget, GL_TEXTURE_WRAP_S, wrapS); - glTexParameteri(renderTarget, GL_TEXTURE_WRAP_T, wrapT); + glTexParameteri(mTarget, GL_TEXTURE_WRAP_S, wrapS); + glTexParameteri(mTarget, GL_TEXTURE_WRAP_T, wrapT); } } -void Texture::setFilterMinMag(GLenum min, GLenum mag, bool bindTexture, bool force, - GLenum renderTarget) { - +void Texture::setFilterMinMag(GLenum min, GLenum mag, bool bindTexture, bool force) { if (force || min != mMinFilter || mag != mMagFilter) { mMinFilter = min; mMagFilter = mag; if (bindTexture) { - mCaches.textureState().bindTexture(renderTarget, mId); + mCaches.textureState().bindTexture(mTarget, mId); } if (mipMap && min == GL_LINEAR) min = GL_LINEAR_MIPMAP_LINEAR; - glTexParameteri(renderTarget, GL_TEXTURE_MIN_FILTER, min); - glTexParameteri(renderTarget, GL_TEXTURE_MAG_FILTER, mag); + glTexParameteri(mTarget, GL_TEXTURE_MIN_FILTER, min); + glTexParameteri(mTarget, GL_TEXTURE_MAG_FILTER, mag); } } @@ -87,15 +84,20 @@ void Texture::deleteTexture() { mId = 0; } -bool Texture::updateSize(uint32_t width, uint32_t height, GLint internalFormat, GLint format) { - if (mWidth == width && mHeight == height && - mFormat == format && mInternalFormat == internalFormat) { +bool Texture::updateSize(uint32_t width, uint32_t height, GLint internalFormat, + GLint format, GLenum target) { + if (mWidth == width + && mHeight == height + && mFormat == format + && mInternalFormat == internalFormat + && mTarget == target) { return false; } mWidth = width; mHeight = height; mFormat = format; mInternalFormat = internalFormat; + mTarget = target; notifySizeChanged(mWidth * mHeight * bytesPerPixel(internalFormat)); return true; } @@ -110,7 +112,7 @@ void Texture::resetCachedParams() { void Texture::upload(GLint internalFormat, uint32_t width, uint32_t height, GLenum format, GLenum type, const void* pixels) { GL_CHECKPOINT(MODERATE); - bool needsAlloc = updateSize(width, height, internalFormat, format); + bool needsAlloc = updateSize(width, height, internalFormat, format, GL_TEXTURE_2D); if (!mId) { glGenTextures(1, &mId); needsAlloc = true; @@ -241,7 +243,7 @@ void Texture::upload(Bitmap& bitmap) { GLint internalFormat, format, type; colorTypeToGlFormatAndType(mCaches, bitmap.colorType(), needSRGB, &internalFormat, &format, &type); - if (updateSize(bitmap.width(), bitmap.height(), internalFormat, format)) { + if (updateSize(bitmap.width(), bitmap.height(), internalFormat, format, GL_TEXTURE_2D)) { needsAlloc = true; } @@ -286,12 +288,14 @@ void Texture::upload(Bitmap& bitmap) { } } -void Texture::wrap(GLuint id, uint32_t width, uint32_t height, GLint internalFormat, GLint format) { +void Texture::wrap(GLuint id, uint32_t width, uint32_t height, + GLint internalFormat, GLint format, GLenum target) { mId = id; mWidth = width; mHeight = height; mFormat = format; mInternalFormat = internalFormat; + mTarget = target; // We're wrapping an existing texture, so don't double count this memory notifySizeChanged(0); } diff --git a/libs/hwui/Texture.h b/libs/hwui/Texture.h index 57cfc2b0a9553..d73789a28ead8 100644 --- a/libs/hwui/Texture.h +++ b/libs/hwui/Texture.h @@ -41,21 +41,19 @@ public: virtual ~Texture() { } - inline void setWrap(GLenum wrap, bool bindTexture = false, bool force = false, - GLenum renderTarget = GL_TEXTURE_2D) { - setWrapST(wrap, wrap, bindTexture, force, renderTarget); + inline void setWrap(GLenum wrap, bool bindTexture = false, bool force = false) { + setWrapST(wrap, wrap, bindTexture, force); } virtual void setWrapST(GLenum wrapS, GLenum wrapT, bool bindTexture = false, - bool force = false, GLenum renderTarget = GL_TEXTURE_2D); + bool force = false); - inline void setFilter(GLenum filter, bool bindTexture = false, bool force = false, - GLenum renderTarget = GL_TEXTURE_2D) { - setFilterMinMag(filter, filter, bindTexture, force, renderTarget); + inline void setFilter(GLenum filter, bool bindTexture = false, bool force = false) { + setFilterMinMag(filter, filter, bindTexture, force); } virtual void setFilterMinMag(GLenum min, GLenum mag, bool bindTexture = false, - bool force = false, GLenum renderTarget = GL_TEXTURE_2D); + bool force = false); /** * Convenience method to call glDeleteTextures() on this texture's id. @@ -91,7 +89,8 @@ public: /** * Wraps an existing texture. */ - void wrap(GLuint id, uint32_t width, uint32_t height, GLint internalFormat, GLint format); + void wrap(GLuint id, uint32_t width, uint32_t height, GLint internalFormat, + GLint format, GLenum target); GLuint id() const { return mId; @@ -113,6 +112,10 @@ public: return mInternalFormat; } + GLenum target() const { + return mTarget; + } + /** * Generation of the backing bitmap, */ @@ -152,7 +155,8 @@ private: friend class Layer; // Returns true if the size changed, false if it was the same - bool updateSize(uint32_t width, uint32_t height, GLint internalFormat, GLint format); + bool updateSize(uint32_t width, uint32_t height, GLint internalFormat, + GLint format, GLenum target); void resetCachedParams(); GLuint mId = 0; @@ -160,6 +164,7 @@ private: uint32_t mHeight = 0; GLint mFormat = 0; GLint mInternalFormat = 0; + GLenum mTarget = GL_NONE; /* See GLES spec section 3.8.14 * "In the initial state, the value assigned to TEXTURE_MIN_FILTER is diff --git a/libs/hwui/renderstate/RenderState.cpp b/libs/hwui/renderstate/RenderState.cpp index 84ab3f31e7abc..72af7c98e4777 100644 --- a/libs/hwui/renderstate/RenderState.cpp +++ b/libs/hwui/renderstate/RenderState.cpp @@ -278,12 +278,12 @@ void RenderState::render(const Glop& glop, const Matrix4& orthoMatrix) { // texture always takes slot 0, shader samplers increment from there mCaches->textureState().activateTexture(0); - mCaches->textureState().bindTexture(texture.target, texture.texture->id()); + mCaches->textureState().bindTexture(texture.texture->target(), texture.texture->id()); if (texture.clamp != GL_INVALID_ENUM) { - texture.texture->setWrap(texture.clamp, false, false, texture.target); + texture.texture->setWrap(texture.clamp, false, false); } if (texture.filter != GL_INVALID_ENUM) { - texture.texture->setFilter(texture.filter, false, false, texture.target); + texture.texture->setFilter(texture.filter, false, false); } if (texture.textureTransform) { diff --git a/libs/hwui/tests/unit/GlopBuilderTests.cpp b/libs/hwui/tests/unit/GlopBuilderTests.cpp index 67e58e2aba0d9..ce1db0585be80 100644 --- a/libs/hwui/tests/unit/GlopBuilderTests.cpp +++ b/libs/hwui/tests/unit/GlopBuilderTests.cpp @@ -45,7 +45,11 @@ static void expectFillEq(Glop::Fill& expectedFill, Glop::Fill& builtFill) { EXPECT_EQ(expectedFill.skiaShaderData.skiaShaderType, builtFill.skiaShaderData.skiaShaderType); EXPECT_EQ(expectedFill.texture.clamp, builtFill.texture.clamp); EXPECT_EQ(expectedFill.texture.filter, builtFill.texture.filter); - EXPECT_EQ(expectedFill.texture.target, builtFill.texture.target); + EXPECT_TRUE((expectedFill.texture.texture && builtFill.texture.texture) + || (!expectedFill.texture.texture && !builtFill.texture.texture)); + if (expectedFill.texture.texture) { + EXPECT_EQ(expectedFill.texture.texture->target(), builtFill.texture.texture->target()); + } EXPECT_EQ(expectedFill.texture.textureTransform, builtFill.texture.textureTransform); } @@ -108,7 +112,7 @@ static std::unique_ptr blackUnitQuadGlop(RenderState& renderState) { glop->fill.color.set(Color::Black); glop->fill.skiaShaderData.skiaShaderType = kNone_SkiaShaderType; glop->fill.filterMode = ProgramDescription::ColorFilterMode::None; - glop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; + glop->fill.texture = { nullptr, GL_INVALID_ENUM, GL_INVALID_ENUM, nullptr }; return glop; }