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 <[email protected]>
Reviewed-by: Adrien Destugues <[email protected]>
This commit is contained in:
Dale Cieslak
2023-02-06 09:21:12 +00:00
committed by Adrien Destugues
parent d778c82da4
commit 4f52a155e6
8 changed files with 52 additions and 116 deletions
+1 -1
View File
@@ -284,7 +284,7 @@ public:
status_t LoadFont(const char* path); status_t LoadFont(const char* path);
status_t LoadFont(const area_id fontAreaID, status_t LoadFont(const area_id fontAreaID,
uint32 size = 0, uint32 offset = 0); size_t size = 0, size_t offset = 0);
status_t UnloadFont(); status_t UnloadFont();
private: private:
+3 -3
View File
@@ -1478,15 +1478,15 @@ BFont::LoadFont(const char* path)
status_t 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; BPrivate::AppServerLink link;
link.StartMessage(AS_ADD_FONT_MEMORY); link.StartMessage(AS_ADD_FONT_MEMORY);
link.Attach<int32>(fontAreaID); link.Attach<int32>(fontAreaID);
link.Attach<uint32>(size); link.Attach<size_t>(size);
link.Attach<uint32>(offset); link.Attach<size_t>(offset);
status_t status = B_ERROR; status_t status = B_ERROR;
if (link.FlushWithReply(status) != B_OK || status != B_OK) { if (link.FlushWithReply(status) != B_OK || status != B_OK) {
+34 -69
View File
@@ -30,6 +30,7 @@
#include <AppDefs.h> #include <AppDefs.h>
#include <Autolock.h> #include <Autolock.h>
#include <AutoLocker.h>
#include <Debug.h> #include <Debug.h>
#include <List.h> #include <List.h>
#include <ScrollBar.h> #include <ScrollBar.h>
@@ -1588,10 +1589,10 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
// 2) uint16 - style ID of added font // 2) uint16 - style ID of added font
// 3) uint16 - face of added font // 3) uint16 - face of added font
fAppFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(fAppFontManager);
if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) { if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) {
fLink.StartMessage(B_NOT_ALLOWED); fLink.StartMessage(B_NOT_ALLOWED);
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
@@ -1603,8 +1604,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
status_t status = fAppFontManager->AddUserFontFromFile(fontPath, status_t status = fAppFontManager->AddUserFontFromFile(fontPath,
familyID, styleID); familyID, styleID);
fAppFontManager->Unlock();
if (status != B_OK) { if (status != B_OK) {
fLink.StartMessage(status); fLink.StartMessage(status);
} else { } else {
@@ -1642,14 +1641,16 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
// Attached Data: // Attached Data:
// 1) area_id - id of memory area where font resides // 1) area_id - id of memory area where font resides
// 2) uint32 - size of memory area for font // 2) size_t - size of memory area for font
// 3) uint32 - offset to start of font memory // 3) size_t - offset to start of font memory
// Returns: // Returns:
// 1) uint16 - family ID of added font // 1) uint16 - family ID of added font
// 2) uint16 - style ID of added font // 2) uint16 - style ID of added font
// 3) uint16 - face of added font // 3) uint16 - face of added font
AutoLocker<FontManagerBase> fontLock(fAppFontManager);
if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) { if (fAppFontManager->CountFamilies() > MAX_USER_FONTS) {
fLink.StartMessage(B_NOT_ALLOWED); fLink.StartMessage(B_NOT_ALLOWED);
fLink.Flush(); fLink.Flush();
@@ -1659,11 +1660,11 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
area_id fontAreaID, fontAreaCloneID; area_id fontAreaID, fontAreaCloneID;
area_info fontAreaInfo; area_info fontAreaInfo;
char* area_addr; char* area_addr;
uint32 size, offset; size_t size, offset;
link.Read<int32>(&fontAreaID); link.Read<int32>(&fontAreaID);
link.Read<uint32>(&size); link.Read<size_t>(&size);
link.Read<uint32>(&offset); link.Read<size_t>(&offset);
fontAreaCloneID = clone_area("user font", fontAreaCloneID = clone_area("user font",
(void **)&area_addr, (void **)&area_addr,
B_ANY_ADDRESS, B_ANY_ADDRESS,
@@ -1684,7 +1685,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
break; break;
} }
uint32 fontMemorySize = fontAreaInfo.size - offset; size_t fontMemorySize = fontAreaInfo.size - offset;
if (size == 0) if (size == 0)
size = fontMemorySize; size = fontMemorySize;
@@ -1712,7 +1713,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
uint16 familyID, styleID; uint16 familyID, styleID;
fAppFontManager->Lock();
status = fAppFontManager->AddUserFontFromMemory(fontData, size, status = fAppFontManager->AddUserFontFromMemory(fontData, size,
familyID, styleID); familyID, styleID);
@@ -1744,7 +1744,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} }
} }
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
@@ -1766,7 +1765,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
status_t status = B_OK; status_t status = B_OK;
fAppFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(fAppFontManager);
FontStyle* style = fAppFontManager->GetStyle(familyID, styleID); FontStyle* style = fAppFontManager->GetStyle(familyID, styleID);
if (style != NULL) { if (style != NULL) {
@@ -1774,8 +1773,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
status = B_BAD_VALUE; status = B_BAD_VALUE;
fAppFontManager->Unlock();
fLink.StartMessage(status); fLink.StartMessage(status);
fLink.Flush(); fLink.Flush();
break; break;
@@ -1952,12 +1949,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
int32 index; int32 index;
link.Read<int32>(&index); link.Read<int32>(&index);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontFamily* family = gFontManager->FamilyAt(index); FontFamily* family = gFontManager->FamilyAt(index);
if (family == NULL) { if (family == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
family = fAppFontManager->FamilyAt(index); family = fAppFontManager->FamilyAt(index);
} }
@@ -1980,10 +1977,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
@@ -2005,12 +1998,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
link.Read<uint16>(&familyID); link.Read<uint16>(&familyID);
link.Read<uint16>(&styleID); link.Read<uint16>(&styleID);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(familyID, styleID); fontStyle = fAppFontManager->GetStyle(familyID, styleID);
} }
@@ -2022,10 +2015,7 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
fLink.Flush(); fLink.Flush();
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
break; break;
} }
@@ -2056,13 +2046,13 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
&& link.Read<uint16>(&styleID) == B_OK && link.Read<uint16>(&styleID) == B_OK
&& link.Read<uint16>(&face) == B_OK) { && link.Read<uint16>(&face) == B_OK) {
// get the font and return IDs and face // get the font and return IDs and face
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(family, style, FontStyle* fontStyle = gFontManager->GetStyle(family, style,
familyID, styleID, face); familyID, styleID, face);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(family, style, fontStyle = fAppFontManager->GetStyle(family, style,
familyID, styleID, face); familyID, styleID, face);
} }
@@ -2078,11 +2068,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
fLink.Attach<uint16>(face); fLink.Attach<uint16>(face);
} else } else
fLink.StartMessage(B_NAME_NOT_FOUND); fLink.StartMessage(B_NAME_NOT_FOUND);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
@@ -2105,12 +2090,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
link.Read<int32>(&familyID); link.Read<int32>(&familyID);
link.Read<int32>(&styleID); link.Read<int32>(&styleID);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(familyID, styleID); fontStyle = fAppFontManager->GetStyle(familyID, styleID);
} }
@@ -2120,11 +2105,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
@@ -2258,12 +2238,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
link.Read<uint16>(&familyID); link.Read<uint16>(&familyID);
link.Read<uint16>(&styleID); link.Read<uint16>(&styleID);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(familyID, styleID); fontStyle = fAppFontManager->GetStyle(familyID, styleID);
} }
@@ -2273,11 +2253,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
@@ -2317,12 +2292,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
link.Read<uint16>(&familyID); link.Read<uint16>(&familyID);
link.Read<uint16>(&styleID); link.Read<uint16>(&styleID);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(familyID, styleID); fontStyle = fAppFontManager->GetStyle(familyID, styleID);
} }
@@ -2332,11 +2307,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
@@ -2356,12 +2326,12 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
link.Read<uint16>(&styleID); link.Read<uint16>(&styleID);
link.Read<float>(&size); link.Read<float>(&size);
gFontManager->Lock(); AutoLocker<FontManagerBase> fontLock(gFontManager);
FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID); FontStyle* fontStyle = gFontManager->GetStyle(familyID, styleID);
if (fontStyle == NULL) { if (fontStyle == NULL) {
gFontManager->Unlock(); fontLock.SetTo(fAppFontManager, false);
fAppFontManager->Lock();
fontStyle = fAppFontManager->GetStyle(familyID, styleID); fontStyle = fAppFontManager->GetStyle(familyID, styleID);
} }
@@ -2374,11 +2344,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link)
} else } else
fLink.StartMessage(B_BAD_VALUE); fLink.StartMessage(B_BAD_VALUE);
if (gFontManager->IsLocked())
gFontManager->Unlock();
if (fAppFontManager->IsLocked())
fAppFontManager->Unlock();
fLink.Flush(); fLink.Flush();
break; break;
} }
+2 -11
View File
@@ -34,7 +34,7 @@
#include "ServerFont.h" #include "ServerFont.h"
#define TRACE_FONT_MANAGER //#define TRACE_FONT_MANAGER
#ifdef TRACE_FONT_MANAGER #ifdef TRACE_FONT_MANAGER
# define FTRACE(x) debug_printf x # define FTRACE(x) debug_printf x
#else #else
@@ -56,7 +56,6 @@ AppFontManager::AppFontManager()
} }
//! Frees all families and styles loaded by the application
AppFontManager::~AppFontManager() AppFontManager::~AppFontManager()
{ {
while (fFamilies.CountItems() > 0) { while (fFamilies.CountItems() > 0) {
@@ -69,20 +68,12 @@ AppFontManager::~AppFontManager()
family->RemoveStyle(styleRef, this); family->RemoveStyle(styleRef, this);
styleRef->ReleaseReference(); styleRef->ReleaseReference();
} }
fFamilies.RemoveItem(family); fFamilies.RemoveItem(family);
delete family; delete family;
} }
} }
void
AppFontManager::MessageReceived(BMessage* message)
{
FontManagerBase::MessageReceived(message);
}
status_t status_t
AppFontManager::_AddUserFont(FT_Face face, node_ref nodeRef, const char* path, AppFontManager::_AddUserFont(FT_Face face, node_ref nodeRef, const char* path,
uint16& familyID, uint16& styleID) 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. /*! \brief Adds the FontFamily/FontStyle that is represented by the area in memory.
*/ */
status_t status_t
AppFontManager::AddUserFontFromMemory(const FT_Byte* fontAddress, uint32 size, AppFontManager::AddUserFontFromMemory(const FT_Byte* fontAddress, size_t size,
uint16& familyID, uint16& styleID) uint16& familyID, uint16& styleID)
{ {
ASSERT(IsLocked()); ASSERT(IsLocked());
+1 -3
View File
@@ -45,12 +45,10 @@ public:
AppFontManager(); AppFontManager();
virtual ~AppFontManager(); virtual ~AppFontManager();
virtual void MessageReceived(BMessage* message);
status_t AddUserFontFromFile(const char* path, status_t AddUserFontFromFile(const char* path,
uint16& familyID, uint16& styleID); uint16& familyID, uint16& styleID);
status_t AddUserFontFromMemory(const FT_Byte* fontAddress, 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); status_t RemoveUserFont(uint16 familyID, uint16 styleID);
private: private:
+7 -1
View File
@@ -135,10 +135,16 @@ bool
FontFamily::RemoveStyle(FontStyle* style, AppFontManager* fontManager) FontFamily::RemoveStyle(FontStyle* style, AppFontManager* fontManager)
{ {
if (!gFontManager->IsLocked() && fontManager == NULL) { 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; return false;
} }
if (style == NULL)
return false;
if (!fStyles.RemoveItem(style)) if (!fStyles.RemoveItem(style))
return false; return false;
+4 -26
View File
@@ -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() FontManagerBase::~FontManagerBase()
{ {
// free families before we're done with FreeType
for (int32 i = fFamilies.CountItems(); i-- > 0;) for (int32 i = fFamilies.CountItems(); i-- > 0;)
delete fFamilies.ItemAt(i); 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 /*! \brief Finds and returns the first valid charmap in a font
\param face Font handle obtained from FT_Load_Face() \param face Font handle obtained from FT_Load_Face()
@@ -145,7 +132,7 @@ int32
FontManagerBase::CountStyles(const char *familyName) FontManagerBase::CountStyles(const char *familyName)
{ {
FontFamily *family = GetFamily(familyName); FontFamily *family = GetFamily(familyName);
if (family) if (family != NULL)
return family->CountStyles(); return family->CountStyles();
return 0; return 0;
@@ -160,7 +147,7 @@ int32
FontManagerBase::CountStyles(uint16 familyID) FontManagerBase::CountStyles(uint16 familyID)
{ {
FontFamily *family = GetFamily(familyID); FontFamily *family = GetFamily(familyID);
if (family) if (family != NULL)
return family->CountStyles(); return family->CountStyles();
return 0; return 0;
@@ -186,10 +173,6 @@ FontManagerBase::GetFamily(const char* name)
if (name == NULL) if (name == NULL)
return NULL; return NULL;
FontFamily* family = _FindFamily(name);
if (family != NULL)
return family;
return _FindFamily(name); return _FindFamily(name);
} }
@@ -275,13 +258,8 @@ FontManagerBase::GetStyle(const char* familyName, const char* styleName,
// find style // find style
if (styleName != NULL && styleName[0]) { if (styleName != NULL && styleName[0])
FontStyle* fontStyle = family->GetStyle(styleName);
if (fontStyle != NULL)
return fontStyle;
return family->GetStyle(styleName); return family->GetStyle(styleName);
}
if (styleID != 0xffff) if (styleID != 0xffff)
return family->GetStyleByID(styleID); return family->GetStyleByID(styleID);
-2
View File
@@ -43,8 +43,6 @@ public:
void SetInitStatus(status_t new_status) void SetInitStatus(status_t new_status)
{ fInitStatus = new_status; } { fInitStatus = new_status; }
virtual void MessageReceived(BMessage* message);
virtual int32 CountFamilies(); virtual int32 CountFamilies();
virtual int32 CountStyles(const char* family); virtual int32 CountStyles(const char* family);