diff options
author | Raph Levien <raph@google.com> | 2014-05-05 16:11:17 -0700 |
---|---|---|
committer | Raph Levien <raph@google.com> | 2014-05-12 10:28:15 -0700 |
commit | b80c1f19c58b927820a8a24bf2218e5645724608 (patch) | |
tree | e00dec46530cfec05ea82512605ff2bf3ff32f28 | |
parent | ecc2d34ac23a497988f21e5f415b53c007b9d8c5 (diff) | |
download | android_frameworks_minikin-b80c1f19c58b927820a8a24bf2218e5645724608.tar.gz android_frameworks_minikin-b80c1f19c58b927820a8a24bf2218e5645724608.tar.bz2 android_frameworks_minikin-b80c1f19c58b927820a8a24bf2218e5645724608.zip |
Better refcounting and locking
All major externally accessible objects (especially FontFamily and
FontCollection) are now reference counted. In addition, there is a
global lock intended to make operations thread-safe.
WIP notice: in this version of the patch, not all external API entry
points are protected by the lock. That should be fixed.
Change-Id: I14106196e99eb101e8bf1bcb4b81359759d2086c
-rw-r--r-- | include/minikin/FontCollection.h | 11 | ||||
-rw-r--r-- | include/minikin/FontFamily.h | 6 | ||||
-rw-r--r-- | include/minikin/MinikinFont.h | 19 | ||||
-rw-r--r-- | include/minikin/MinikinRefCounted.h | 42 | ||||
-rw-r--r-- | libs/minikin/Android.mk | 2 | ||||
-rw-r--r-- | libs/minikin/FontCollection.cpp | 6 | ||||
-rw-r--r-- | libs/minikin/FontFamily.cpp | 7 | ||||
-rw-r--r-- | libs/minikin/Layout.cpp | 7 | ||||
-rw-r--r-- | libs/minikin/MinikinInternal.cpp | 25 | ||||
-rw-r--r-- | libs/minikin/MinikinInternal.h | 34 | ||||
-rw-r--r-- | libs/minikin/MinikinRefCounted.cpp | 35 |
11 files changed, 164 insertions, 30 deletions
diff --git a/include/minikin/FontCollection.h b/include/minikin/FontCollection.h index a2a5391..dc48f8e 100644 --- a/include/minikin/FontCollection.h +++ b/include/minikin/FontCollection.h @@ -19,13 +19,14 @@ #include <vector> +#include <minikin/MinikinRefCounted.h> #include <minikin/MinikinFont.h> #include <minikin/SparseBitSet.h> #include <minikin/FontFamily.h> namespace android { -class FontCollection { +class FontCollection : public MinikinRefCounted { public: explicit FontCollection(const std::vector<FontFamily*>& typefaces); @@ -37,17 +38,17 @@ public: // Do copy constructor, assignment, destructor so it can be used in vectors Run() : font(NULL) { } Run(const Run& other): font(other.font), start(other.start), end(other.end) { - if (font) font->Ref(); + if (font) font->RefLocked(); } Run& operator=(const Run& other) { - if (other.font) other.font->Ref(); - if (font) font->Unref(); + if (other.font) other.font->RefLocked(); + if (font) font->UnrefLocked(); font = other.font; start = other.start; end = other.end; return *this; } - ~Run() { if (font) font->Unref(); } + ~Run() { if (font) font->UnrefLocked(); } MinikinFont* font; int start; diff --git a/include/minikin/FontFamily.h b/include/minikin/FontFamily.h index 290220b..3b59017 100644 --- a/include/minikin/FontFamily.h +++ b/include/minikin/FontFamily.h @@ -19,6 +19,8 @@ #include <vector> +#include <minikin/MinikinRefCounted.h> + namespace android { // FontStyle represents all style information needed to select an actual font @@ -39,8 +41,10 @@ private: uint32_t bits; }; -class FontFamily { +class FontFamily : public MinikinRefCounted { public: + ~FontFamily(); + // Add font to family, extracting style information from the font bool addFont(MinikinFont* typeface); diff --git a/include/minikin/MinikinFont.h b/include/minikin/MinikinFont.h index e84f5df..568f19d 100644 --- a/include/minikin/MinikinFont.h +++ b/include/minikin/MinikinFont.h @@ -17,6 +17,8 @@ #ifndef MINIKIN_FONT_H #define MINIKIN_FONT_H +#include <minikin/MinikinRefCounted.h> + // An abstraction for platform fonts, allowing Minikin to be used with // multiple actual implementations of fonts. @@ -56,15 +58,8 @@ struct MinikinRect { class MinikinFontFreeType; -class MinikinFont { +class MinikinFont : public MinikinRefCounted { public: - void Ref() { mRefcount_++; } - void Unref() { if (--mRefcount_ == 0) { delete this; } } - - MinikinFont() : mRefcount_(1) { } - - virtual ~MinikinFont() { }; - virtual bool GetGlyph(uint32_t codepoint, uint32_t *glyph) const = 0; virtual float GetHorizontalAdvance(uint32_t glyph_id, @@ -82,14 +77,6 @@ public: return ((uint32_t)c1 << 24) | ((uint32_t)c2 << 16) | ((uint32_t)c3 << 8) | (uint32_t)c4; } - - // This is used to implement a downcast without RTTI - virtual MinikinFontFreeType* GetFreeType() { - return NULL; - } - -private: - int mRefcount_; }; } // namespace android diff --git a/include/minikin/MinikinRefCounted.h b/include/minikin/MinikinRefCounted.h new file mode 100644 index 0000000..74d27fe --- /dev/null +++ b/include/minikin/MinikinRefCounted.h @@ -0,0 +1,42 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Base class for reference counted objects in Minikin + +#ifndef MINIKIN_REF_COUNTED_H +#define MINIKIN_REF_COUNTED_H + +namespace android { + +class MinikinRefCounted { +public: + void RefLocked() { mRefcount_++; } + void UnrefLocked() { if (--mRefcount_ == 0) { delete this; } } + + // These refcount operations take the global lock. + void Ref(); + void Unref(); + + MinikinRefCounted() : mRefcount_(1) { } + + virtual ~MinikinRefCounted() { }; +private: + int mRefcount_; +}; + +} + +#endif // MINIKIN_REF_COUNTED_H
\ No newline at end of file diff --git a/libs/minikin/Android.mk b/libs/minikin/Android.mk index 5e13495..c441285 100644 --- a/libs/minikin/Android.mk +++ b/libs/minikin/Android.mk @@ -24,6 +24,8 @@ LOCAL_SRC_FILES := \ FontCollection.cpp \ FontFamily.cpp \ Layout.cpp \ + MinikinInternal.cpp \ + MinikinRefCounted.cpp \ MinikinFontFreeType.cpp \ SparseBitSet.cpp diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp index aa37825..18e528e 100644 --- a/libs/minikin/FontCollection.cpp +++ b/libs/minikin/FontCollection.cpp @@ -41,6 +41,7 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) : const FontStyle defaultStyle; for (size_t i = 0; i < nTypefaces; i++) { FontFamily* family = typefaces[i]; + family->RefLocked(); FontInstance dummy; mInstances.push_back(dummy); // emplace_back would be better FontInstance* instance = &mInstances.back(); @@ -62,7 +63,6 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) : #endif mMaxChar = max(mMaxChar, instance->mCoverage->length()); lastChar.push_back(instance->mCoverage->nextSetBit(0)); - // TODO: should probably ref typeface here, hmm } size_t nPages = mMaxChar >> kLogCharsPerPage; size_t offset = 0; @@ -93,7 +93,7 @@ FontCollection::FontCollection(const vector<FontFamily*>& typefaces) : FontCollection::~FontCollection() { for (size_t i = 0; i < mInstances.size(); i++) { delete mInstances[i].mCoverage; - // probably unref the typeface here too + mInstances[i].mFamily->UnrefLocked(); } } @@ -138,7 +138,7 @@ void FontCollection::itemize(const uint16_t *string, size_t string_size, FontSty run->font = NULL; // maybe we should do something different here } else { run->font = family->getClosestMatch(style); - run->font->Ref(); + run->font->RefLocked(); } lastFamily = family; run->start = i; diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp index 16031ce..d8525ff 100644 --- a/libs/minikin/FontFamily.cpp +++ b/libs/minikin/FontFamily.cpp @@ -28,6 +28,12 @@ using std::vector; namespace android { +FontFamily::~FontFamily() { + for (size_t i = 0; i < mFonts.size(); i++) { + mFonts[i].typeface->UnrefLocked(); + } +} + bool FontFamily::addFont(MinikinFont* typeface) { const uint32_t os2Tag = MinikinFont::MakeTag('O', 'S', '/', '2'); size_t os2Size = 0; @@ -50,6 +56,7 @@ bool FontFamily::addFont(MinikinFont* typeface) { } void FontFamily::addFont(MinikinFont* typeface, FontStyle style) { + typeface->RefLocked(); mFonts.push_back(Font(typeface, style)); } diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp index ffaa451..20e2d9e 100644 --- a/libs/minikin/Layout.cpp +++ b/libs/minikin/Layout.cpp @@ -25,8 +25,7 @@ #include <hb-icu.h> -#include <utils/Mutex.h> - +#include "MinikinInternal.h" #include <minikin/MinikinFontFreeType.h> #include <minikin/Layout.h> @@ -38,8 +37,6 @@ namespace android { // TODO: globals are not cool, move to a factory-ish object hb_buffer_t* buffer = 0; -Mutex gLock; - Bitmap::Bitmap(int width, int height) : width(width), height(height) { buf = new uint8_t[width * height](); } @@ -280,7 +277,7 @@ static hb_script_t getScriptRun(const uint16_t *chars, size_t len, ssize_t *iter // TODO: API should probably take context void Layout::doLayout(const uint16_t* buf, size_t nchars) { - AutoMutex _l(gLock); + AutoMutex _l(gMinikinLock); if (buffer == 0) { buffer = hb_buffer_create(); } diff --git a/libs/minikin/MinikinInternal.cpp b/libs/minikin/MinikinInternal.cpp new file mode 100644 index 0000000..71c8649 --- /dev/null +++ b/libs/minikin/MinikinInternal.cpp @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Definitions internal to Minikin + +#include "MinikinInternal.h" + +namespace android { + +Mutex gMinikinLock; + +} diff --git a/libs/minikin/MinikinInternal.h b/libs/minikin/MinikinInternal.h new file mode 100644 index 0000000..b8430df --- /dev/null +++ b/libs/minikin/MinikinInternal.h @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Definitions internal to Minikin + +#ifndef MINIKIN_INTERNAL_H +#define MINIKIN_INTERNAL_H + +#include <utils/Mutex.h> + +namespace android { + +// All external Minikin interfaces are designed to be thread-safe. +// Presently, that's implemented by through a global lock, and having +// all external interfaces take that lock. + +extern Mutex gMinikinLock; + +} + +#endif // MINIKIN_INTERNAL_H diff --git a/libs/minikin/MinikinRefCounted.cpp b/libs/minikin/MinikinRefCounted.cpp new file mode 100644 index 0000000..9fa3ae4 --- /dev/null +++ b/libs/minikin/MinikinRefCounted.cpp @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Base class for reference counted objects in Minikin + +#include "MinikinInternal.h" + +#include <minikin/MinikinRefCounted.h> + +namespace android { + +void MinikinRefCounted::Ref() { + AutoMutex _l(gMinikinLock); + this->RefLocked(); +} + +void MinikinRefCounted::Unref() { + AutoMutex _l(gMinikinLock); + this->UnrefLocked(); +} + +} |