From 4f52a155e662513531292fb573bcab89a95e8b44 Mon Sep 17 00:00:00 2001 From: Dale Cieslak Date: Mon, 16 Jan 2023 20:38:33 -0800 Subject: [PATCH] BFont: Minor code cleanup and autolocking for AppFontManager * changed explicit locking to use Autolocker for gFontManager/fAppFontManager in ServerApp, per comments in https://review.haiku-os.org/c/haiku/+/4790 * changed BFont::LoadFont (memory version) to use size_t for size and offset * no functional changes Change-Id: I438a4975d5bb1b2fa17bc54e9e171c31dadfeec5 Reviewed-on: https://review.haiku-os.org/c/haiku/+/6003 Tested-by: Commit checker robot Reviewed-by: Adrien Destugues --- headers/os/interface/Font.h | 2 +- src/kits/interface/Font.cpp | 6 +- src/servers/app/ServerApp.cpp | 103 ++++++++---------------- src/servers/app/font/AppFontManager.cpp | 13 +-- src/servers/app/font/AppFontManager.h | 4 +- src/servers/app/font/FontFamily.cpp | 8 +- src/servers/app/font/FontManager.cpp | 30 +------ src/servers/app/font/FontManager.h | 2 - 8 files changed, 52 insertions(+), 116 deletions(-) diff --git a/headers/os/interface/Font.h b/headers/os/interface/Font.h index 53219c2228..ed93e75d02 100644 --- a/headers/os/interface/Font.h +++ b/headers/os/interface/Font.h @@ -284,7 +284,7 @@ public: status_t LoadFont(const char* path); status_t LoadFont(const area_id fontAreaID, - uint32 size = 0, uint32 offset = 0); + size_t size = 0, size_t offset = 0); status_t UnloadFont(); private: diff --git a/src/kits/interface/Font.cpp b/src/kits/interface/Font.cpp index 58a2cf5d7c..4553a2ee2c 100644 --- a/src/kits/interface/Font.cpp +++ b/src/kits/interface/Font.cpp @@ -1478,15 +1478,15 @@ BFont::LoadFont(const char* path) status_t -BFont::LoadFont(const area_id fontAreaID, uint32 size, uint32 offset) +BFont::LoadFont(const area_id fontAreaID, size_t size, size_t offset) { BPrivate::AppServerLink link; link.StartMessage(AS_ADD_FONT_MEMORY); link.Attach(fontAreaID); - link.Attach(size); - link.Attach(offset); + link.Attach(size); + link.Attach(offset); status_t status = B_ERROR; if (link.FlushWithReply(status) != B_OK || status != B_OK) { diff --git a/src/servers/app/ServerApp.cpp b/src/servers/app/ServerApp.cpp index 16fc6d0188..e6c42e22a9 100644 --- a/src/servers/app/ServerApp.cpp +++ b/src/servers/app/ServerApp.cpp @@ -30,6 +30,7 @@ #include #include +#include #include #include #include @@ -1588,10 +1589,10 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) // 2) uint16 - style ID of added font // 3) uint16 - face of added font - fAppFontManager->Lock(); + AutoLocker fontLock(fAppFontManager); + if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) { fLink.StartMessage(B_NOT_ALLOWED); - fAppFontManager->Unlock(); fLink.Flush(); break; } @@ -1603,8 +1604,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) status_t status = fAppFontManager->AddUserFontFromFile(fontPath, familyID, styleID); - fAppFontManager->Unlock(); - if (status != B_OK) { fLink.StartMessage(status); } else { @@ -1642,14 +1641,16 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) // Attached Data: // 1) area_id - id of memory area where font resides - // 2) uint32 - size of memory area for font - // 3) uint32 - offset to start of font memory + // 2) size_t - size of memory area for font + // 3) size_t - offset to start of font memory // Returns: // 1) uint16 - family ID of added font // 2) uint16 - style ID of added font // 3) uint16 - face of added font + AutoLocker fontLock(fAppFontManager); + if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) { fLink.StartMessage(B_NOT_ALLOWED); fLink.Flush(); @@ -1659,11 +1660,11 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) area_id fontAreaID, fontAreaCloneID; area_info fontAreaInfo; char* area_addr; - uint32 size, offset; + size_t size, offset; link.Read(&fontAreaID); - link.Read(&size); - link.Read(&offset); + link.Read(&size); + link.Read(&offset); fontAreaCloneID = clone_area("user font", (void **)&area_addr, B_ANY_ADDRESS, @@ -1684,7 +1685,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) break; } - uint32 fontMemorySize = fontAreaInfo.size - offset; + size_t fontMemorySize = fontAreaInfo.size - offset; if (size == 0) size = fontMemorySize; @@ -1712,7 +1713,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) uint16 familyID, styleID; - fAppFontManager->Lock(); status = fAppFontManager->AddUserFontFromMemory(fontData, size, familyID, styleID); @@ -1744,7 +1744,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } } - fAppFontManager->Unlock(); fLink.Flush(); break; } @@ -1766,7 +1765,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) status_t status = B_OK; - fAppFontManager->Lock(); + AutoLocker fontLock(fAppFontManager); FontStyle* style = fAppFontManager->GetStyle(familyID, styleID); if (style != NULL) { @@ -1774,8 +1773,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else status = B_BAD_VALUE; - fAppFontManager->Unlock(); - fLink.StartMessage(status); fLink.Flush(); break; @@ -1952,12 +1949,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) int32 index; link.Read(&index); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontFamily* family = gFontManager->FamilyAt(index); if (family == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + family = fAppFontManager->FamilyAt(index); } @@ -1980,10 +1977,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(B_BAD_VALUE); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); fLink.Flush(); break; @@ -2005,12 +1998,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&familyID); link.Read(&styleID); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(familyID, styleID); } @@ -2022,10 +2015,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) fLink.StartMessage(B_BAD_VALUE); fLink.Flush(); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); + break; } @@ -2056,13 +2046,13 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) && link.Read(&styleID) == B_OK && link.Read(&face) == B_OK) { // get the font and return IDs and face - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(family, style, familyID, styleID, face); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(family, style, familyID, styleID, face); } @@ -2078,11 +2068,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) fLink.Attach(face); } else fLink.StartMessage(B_NAME_NOT_FOUND); - - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); } else fLink.StartMessage(B_BAD_VALUE); @@ -2105,12 +2090,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&familyID); link.Read(&styleID); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(familyID, styleID); } @@ -2120,11 +2105,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(B_BAD_VALUE); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); - fLink.Flush(); break; } @@ -2258,12 +2238,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&familyID); link.Read(&styleID); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(familyID, styleID); } @@ -2273,11 +2253,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(B_BAD_VALUE); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); - fLink.Flush(); break; } @@ -2317,12 +2292,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&familyID); link.Read(&styleID); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(familyID, styleID); } @@ -2332,11 +2307,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(B_BAD_VALUE); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); - fLink.Flush(); break; } @@ -2356,12 +2326,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&styleID); link.Read(&size); - gFontManager->Lock(); + AutoLocker fontLock(gFontManager); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); if (fontStyle == NULL) { - gFontManager->Unlock(); - fAppFontManager->Lock(); + fontLock.SetTo(fAppFontManager, false); + fontStyle = fAppFontManager->GetStyle(familyID, styleID); } @@ -2374,11 +2344,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(B_BAD_VALUE); - if (gFontManager->IsLocked()) - gFontManager->Unlock(); - if (fAppFontManager->IsLocked()) - fAppFontManager->Unlock(); - fLink.Flush(); break; } diff --git a/src/servers/app/font/AppFontManager.cpp b/src/servers/app/font/AppFontManager.cpp index 4f9fe0ba23..a06fdb4f4e 100644 --- a/src/servers/app/font/AppFontManager.cpp +++ b/src/servers/app/font/AppFontManager.cpp @@ -34,7 +34,7 @@ #include "ServerFont.h" -#define TRACE_FONT_MANAGER +//#define TRACE_FONT_MANAGER #ifdef TRACE_FONT_MANAGER # define FTRACE(x) debug_printf x #else @@ -56,7 +56,6 @@ AppFontManager::AppFontManager() } -//! Frees all families and styles loaded by the application AppFontManager::~AppFontManager() { while (fFamilies.CountItems() > 0) { @@ -69,20 +68,12 @@ AppFontManager::~AppFontManager() family->RemoveStyle(styleRef, this); styleRef->ReleaseReference(); } - fFamilies.RemoveItem(family); delete family; } } -void -AppFontManager::MessageReceived(BMessage* message) -{ - FontManagerBase::MessageReceived(message); -} - - status_t AppFontManager::_AddUserFont(FT_Face face, node_ref nodeRef, const char* path, uint16& familyID, uint16& styleID) @@ -159,7 +150,7 @@ AppFontManager::AddUserFontFromFile(const char* path, /*! \brief Adds the FontFamily/FontStyle that is represented by the area in memory. */ status_t -AppFontManager::AddUserFontFromMemory(const FT_Byte* fontAddress, uint32 size, +AppFontManager::AddUserFontFromMemory(const FT_Byte* fontAddress, size_t size, uint16& familyID, uint16& styleID) { ASSERT(IsLocked()); diff --git a/src/servers/app/font/AppFontManager.h b/src/servers/app/font/AppFontManager.h index 9b73f2ae92..66a797a798 100644 --- a/src/servers/app/font/AppFontManager.h +++ b/src/servers/app/font/AppFontManager.h @@ -45,12 +45,10 @@ public: AppFontManager(); virtual ~AppFontManager(); - virtual void MessageReceived(BMessage* message); - status_t AddUserFontFromFile(const char* path, uint16& familyID, uint16& styleID); status_t AddUserFontFromMemory(const FT_Byte* fontAddress, - uint32 size, uint16& familyID, uint16& styleID); + size_t size, uint16& familyID, uint16& styleID); status_t RemoveUserFont(uint16 familyID, uint16 styleID); private: diff --git a/src/servers/app/font/FontFamily.cpp b/src/servers/app/font/FontFamily.cpp index 041495e81e..1af8024fdd 100644 --- a/src/servers/app/font/FontFamily.cpp +++ b/src/servers/app/font/FontFamily.cpp @@ -135,10 +135,16 @@ bool FontFamily::RemoveStyle(FontStyle* style, AppFontManager* fontManager) { if (!gFontManager->IsLocked() && fontManager == NULL) { - debugger("FontFamily::RemoveStyle() called without having the font manager locked!"); + debugger("FontFamily::RemoveStyle() called without having the global font manager locked!"); + return false; + } else if (fontManager != NULL && !fontManager->IsLocked()) { + debugger("FontFamily::RemoveStyle() called without having the app font manager locked!"); return false; } + if (style == NULL) + return false; + if (!fStyles.RemoveItem(style)) return false; diff --git a/src/servers/app/font/FontManager.cpp b/src/servers/app/font/FontManager.cpp index 2aa000a102..4b307b4257 100644 --- a/src/servers/app/font/FontManager.cpp +++ b/src/servers/app/font/FontManager.cpp @@ -63,11 +63,9 @@ FontManagerBase::FontManagerBase(bool init_freetype, const char* className) } -//! Frees font families shuts down FreeType if it was initialized +//! Shuts down FreeType if it was initialized FontManagerBase::~FontManagerBase() { - // free families before we're done with FreeType - for (int32 i = fFamilies.CountItems(); i-- > 0;) delete fFamilies.ItemAt(i); @@ -76,17 +74,6 @@ FontManagerBase::~FontManagerBase() } -void -FontManagerBase::MessageReceived(BMessage* message) -{ - switch (message->what) { - default: - BLooper::MessageReceived(message); - break; - } -} - - /*! \brief Finds and returns the first valid charmap in a font \param face Font handle obtained from FT_Load_Face() @@ -145,7 +132,7 @@ int32 FontManagerBase::CountStyles(const char *familyName) { FontFamily *family = GetFamily(familyName); - if (family) + if (family != NULL) return family->CountStyles(); return 0; @@ -160,7 +147,7 @@ int32 FontManagerBase::CountStyles(uint16 familyID) { FontFamily *family = GetFamily(familyID); - if (family) + if (family != NULL) return family->CountStyles(); return 0; @@ -186,10 +173,6 @@ FontManagerBase::GetFamily(const char* name) if (name == NULL) return NULL; - FontFamily* family = _FindFamily(name); - if (family != NULL) - return family; - return _FindFamily(name); } @@ -275,13 +258,8 @@ FontManagerBase::GetStyle(const char* familyName, const char* styleName, // find style - if (styleName != NULL && styleName[0]) { - FontStyle* fontStyle = family->GetStyle(styleName); - if (fontStyle != NULL) - return fontStyle; - + if (styleName != NULL && styleName[0]) return family->GetStyle(styleName); - } if (styleID != 0xffff) return family->GetStyleByID(styleID); diff --git a/src/servers/app/font/FontManager.h b/src/servers/app/font/FontManager.h index ddd42fae96..3bfd2223cd 100644 --- a/src/servers/app/font/FontManager.h +++ b/src/servers/app/font/FontManager.h @@ -43,8 +43,6 @@ public: void SetInitStatus(status_t new_status) { fInitStatus = new_status; } - virtual void MessageReceived(BMessage* message); - virtual int32 CountFamilies(); virtual int32 CountStyles(const char* family);