Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2cb0826d authored by Mike Reed's avatar Mike Reed
Browse files

lockPixels is no longer virtual 

See skbug.com/6481

With pixelrefs now always (logically) locked, we no longer override
onNewLockPixels/onUnlockPixels. We also must now pass our
address/rowBytes to the base constructor.

These changes allow us to remove duplicate fields in the hwui Bitmap
subclass -- mRowBytes and mColorTable, since these fields are now always
available from the base-class.

However, since this subclass still wants to overwrite const fields in
the base-class (its reconfigure api), change the impl to use the newly
added private method (android_only_reset) to encapsulate this.

Test: hwui_unit_tests and CtsGraphicsTestCases pass

Change-Id: I57a3af1135580513b708f35549a6defb7ac6c04e
parent 9c58d46e

libs/hwui/hwui/Bitmap.cpp

+16 −31
Original line number Diff line number Diff line
@@ -323,19 +323,13 @@ sk_sp<Bitmap> Bitmap::createFrom(sp<GraphicBuffer> graphicBuffer) { 
}



void Bitmap::setColorSpace(sk_sp<SkColorSpace> colorSpace) {

    // TODO: See todo in reconfigure() below

    SkImageInfo* myInfo = const_cast<SkImageInfo*>(&this->info());

    *myInfo = info().makeColorSpace(std::move(colorSpace));

    reconfigure(info().makeColorSpace(std::move(colorSpace)), rowBytes(), colorTable());

}



void Bitmap::reconfigure(const SkImageInfo& newInfo, size_t rowBytes, SkColorTable* ctable) {

    if (kIndex_8_SkColorType != newInfo.colorType()) {

        ctable = nullptr;

    }

    mRowBytes = rowBytes;

    if (mColorTable.get() != ctable) {

        mColorTable.reset(SkSafeRef(ctable));

    }



    // Need to validate the alpha type to filter against the color type

    // to prevent things like a non-opaque RGB565 bitmap
@@ -349,50 +343,48 @@ void Bitmap::reconfigure(const SkImageInfo& newInfo, size_t rowBytes, SkColorTab 
    // really hard to work with. Skia really, really wants immutable objects,

    // but with the nested-ref-count hackery going on that's just not

    // feasible without going insane trying to figure it out

    SkImageInfo* myInfo = const_cast<SkImageInfo*>(&this->info());

    *myInfo = newInfo;

    changeAlphaType(alphaType);



    // Docs say to only call this in the ctor, but we're going to call

    // it anyway even if this isn't always the ctor.

    // TODO: Fix this too as part of the above TODO

    setPreLocked(getStorage(), mRowBytes, mColorTable.get());

    this->android_only_reset(newInfo.makeAlphaType(alphaType), rowBytes, sk_ref_sp(ctable));

}



static sk_sp<SkColorTable> sanitize(const SkImageInfo& info, SkColorTable* ctable) {

    if (info.colorType() == kIndex_8_SkColorType) {

        SkASSERT(ctable);

        return sk_ref_sp(ctable);

    }

    return nullptr; // drop the ctable if we're not indexed

}

Bitmap::Bitmap(void* address, size_t size, const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable)

            : SkPixelRef(info)

            : SkPixelRef(info, address, rowBytes, sanitize(info, ctable))

            , mPixelStorageType(PixelStorageType::Heap) {

    mPixelStorage.heap.address = address;

    mPixelStorage.heap.size = size;

    reconfigure(info, rowBytes, ctable);

}



Bitmap::Bitmap(void* address, void* context, FreeFunc freeFunc,

                const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable)

            : SkPixelRef(info)

            : SkPixelRef(info, address, rowBytes, sanitize(info, ctable))

            , mPixelStorageType(PixelStorageType::External) {

    mPixelStorage.external.address = address;

    mPixelStorage.external.context = context;

    mPixelStorage.external.freeFunc = freeFunc;

    reconfigure(info, rowBytes, ctable);

}



Bitmap::Bitmap(void* address, int fd, size_t mappedSize,

                const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable)

            : SkPixelRef(info)

            : SkPixelRef(info, address, rowBytes, sanitize(info, ctable))

            , mPixelStorageType(PixelStorageType::Ashmem) {

    mPixelStorage.ashmem.address = address;

    mPixelStorage.ashmem.fd = fd;

    mPixelStorage.ashmem.size = mappedSize;

    reconfigure(info, rowBytes, ctable);

}



Bitmap::Bitmap(GraphicBuffer* buffer, const SkImageInfo& info)

        : SkPixelRef(info)

        : SkPixelRef(info, nullptr,

                     bytesPerPixel(buffer->getPixelFormat()) * buffer->getStride(),

                     nullptr)

        , mPixelStorageType(PixelStorageType::Hardware) {

    mPixelStorage.hardware.buffer = buffer;

    buffer->incStrong(buffer);

    mRowBytes = bytesPerPixel(buffer->getPixelFormat()) * buffer->getStride();

}



Bitmap::~Bitmap() {
@@ -442,15 +434,8 @@ void* Bitmap::getStorage() const { 
    }

}



bool Bitmap::onNewLockPixels(LockRec* rec) {

    rec->fPixels = getStorage();

    rec->fRowBytes = mRowBytes;

    rec->fColorTable = mColorTable.get();

    return true;

}



size_t Bitmap::getAllocatedSizeInBytes() const {

    return info().getSafeSize(mRowBytes);

    return info().getSafeSize(this->rowBytes());

}



int Bitmap::getAshmemFd() const {

libs/hwui/hwui/Bitmap.h

+2 −13
Original line number Diff line number Diff line
@@ -70,15 +70,8 @@ public: 
    int width() const { return info().width(); }

    int height() const { return info().height(); }



    // Can't mark as override since SkPixelRef::rowBytes isn't virtual

    // but that's OK since we just want Bitmap to be able to rely

    // on calling rowBytes() on an unlocked pixelref, which it will be

    // 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();

        return rowBytes() >> info().shiftPerPixel();

    }



    void reconfigure(const SkImageInfo& info, size_t rowBytes, SkColorTable* ctable);
@@ -103,7 +96,7 @@ public: 
    void getBounds(SkRect* bounds) const;



    bool readyToDraw() const {

        return this->colorType() != kIndex_8_SkColorType || mColorTable;

        return this->colorType() != kIndex_8_SkColorType || this->colorTable();

    }



    bool isHardware() const {
@@ -112,8 +105,6 @@ public: 


    GraphicBuffer* graphicBuffer();

protected:

    virtual bool onNewLockPixels(LockRec* rec) override;

    virtual void onUnlockPixels() override { };

    virtual size_t getAllocatedSizeInBytes() const override;

private:

    Bitmap(GraphicBuffer* buffer, const SkImageInfo& info);
@@ -122,8 +113,6 @@ private: 


    const PixelStorageType mPixelStorageType;



    size_t mRowBytes = 0;

    sk_sp<SkColorTable> mColorTable;

    bool mHasHardwareMipMap = false;



    union {