Fix drawing artefacts in Locale prefs (and IMHO improve the look):

* separate LanguageListItemWithFlag from LanguageListItem
* draw the flag in front of the text instead of at wherever the right bounds happen to
  be, fixing the drawing artefacts when scrolling
* size the flag to match the size of the list item, which looks much better when
  using a largish default font
* use StringItem::BaselineOffset() instead of manually computed (and wrong) offset
  when drawing the text


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@42667 a95241bf-73f2-0310-859d-f6bbb57e9c96

src/preferences/locale/LanguageListView.cpp

@@ -30,47 +30,40 @@
 #define B_TRANSLATE_CONTEXT "LanguageListView"
 
 
-static const float kFlagWidth = 17.0;
+static const float kLeftInset = 4;
 
 LanguageListItem::LanguageListItem(const char* text, const char* id,
-	const char* code, const char* countryCode)
+	const char* languageCode)
 	:
 	BStringItem(text),
 	fID(id),
-	fCode(code)
+	fCode(languageCode)
 {
-	fIcon = new(std::nothrow) BBitmap(BRect(0, 0, 15, 15), B_RGBA32);
-	if (fIcon != NULL && BLocaleRoster::Default()->GetFlagIconForCountry(fIcon,
-			countryCode) != B_OK) {
-		delete fIcon;
-		fIcon = NULL;
-	}
 }
 
 
 LanguageListItem::LanguageListItem(const LanguageListItem& other)
 	:
 	BStringItem(other.Text()),
-	fID(other.ID()),
+	fID(other.fID),
-	fCode(other.Code()),
+	fCode(other.fCode)
-	fIcon(NULL)
 {
-	if (other.fIcon != NULL)
-		fIcon = new BBitmap(*other.fIcon);
-}
-
-
-LanguageListItem::~LanguageListItem()
-{
-	delete fIcon;
 }
 
 
 void
 LanguageListItem::DrawItem(BView* owner, BRect frame, bool complete)
 {
-	rgb_color kHighlight = {140, 140, 140, 0};
+	DrawItemWithTextOffset(owner, frame, complete, 0);
-	rgb_color kBlack = {0, 0, 0, 0};
+}
+
+
+void
+LanguageListItem::DrawItemWithTextOffset(BView* owner, BRect frame,
+	bool complete, float textOffset)
+{
+	static rgb_color kHighlight = {140, 140, 140, 0};
+	static rgb_color kBlack = {0, 0, 0, 0};
 
 	if (IsSelected() || complete) {
 		rgb_color color;
@@ -90,41 +83,81 @@ LanguageListItem::DrawItem(BView* owner, BRect frame, bool complete)
 		owner->SetHighColor(kBlack);
 	else {
 		owner->SetHighColor(tint_color(owner->LowColor(), B_DARKEN_3_TINT));
-		text += "   [";
+		text << "   [" << B_TRANSLATE("already chosen") << "]";
-		text += B_TRANSLATE("already chosen");
-		text += "]";
 	}
 
-	BFont font = be_plain_font;
+	owner->MovePenTo(frame.left + kLeftInset + textOffset,
-	font_height	finfo;
+		frame.top + BaselineOffset());
-	font.GetHeight(&finfo);
-	owner->SetFont(&font);
-	// TODO: the position is unnecessarily complicated, and not correct either
-	owner->MovePenTo(frame.left + 8, frame.top
-		+ (frame.Height() - (finfo.ascent + finfo.descent + finfo.leading)) / 2
-		+ (finfo.ascent + finfo.descent) - 1);
 	owner->DrawString(text.String());
+}
 
-	// Draw the icon
-	frame.left = frame.right - kFlagWidth;
-	BRect iconFrame(frame);
-	iconFrame.Set(iconFrame.left, iconFrame.top + 1, iconFrame.left + kFlagWidth - 2,
-		iconFrame.top + kFlagWidth - 1);
 
-	if (fIcon != NULL && fIcon->IsValid()) {
+// #pragma mark -
-		owner->SetDrawingMode(B_OP_OVER);
-		owner->DrawBitmap(fIcon, iconFrame);
-		owner->SetDrawingMode(B_OP_COPY);
-	}
 
+
+LanguageListItemWithFlag::LanguageListItemWithFlag(const char* text,
+	const char* id, const char* languageCode, const char* countryCode)
+	:
+	LanguageListItem(text, id, languageCode),
+	fCountryCode(countryCode),
+	fIcon(NULL)
+{
+}
+
+
+LanguageListItemWithFlag::LanguageListItemWithFlag(
+	const LanguageListItemWithFlag& other)
+	:
+	LanguageListItem(other),
+	fCountryCode(other.fCountryCode),
+	fIcon(other.fIcon != NULL ? new BBitmap(*other.fIcon) : NULL)
+{
+}
+
+
+LanguageListItemWithFlag::~LanguageListItemWithFlag()
+{
+	delete fIcon;
 }
 
 
 void
-LanguageListItem::Update(BView* owner, const BFont* font)
+LanguageListItemWithFlag::Update(BView* owner, const BFont* font)
 {
-	BStringItem::Update(owner, font);
+	LanguageListItem::Update(owner, font);
-	SetWidth(Width() + kFlagWidth);
+
+	float iconSize = Height();
+	SetWidth(Width() + iconSize + 4);
+
+	if (fCountryCode.IsEmpty())
+		return;
+
+	fIcon = new(std::nothrow) BBitmap(BRect(0, 0, iconSize - 1, iconSize - 1),
+		B_RGBA32);
+	if (fIcon != NULL && BLocaleRoster::Default()->GetFlagIconForCountry(fIcon,
+			fCountryCode.String()) != B_OK) {
+		delete fIcon;
+		fIcon = NULL;
+	}
+}
+
+
+void
+LanguageListItemWithFlag::DrawItem(BView* owner, BRect frame, bool complete)
+{
+	if (fIcon == NULL || !fIcon->IsValid()) {
+		DrawItemWithTextOffset(owner, frame, complete, 0);
+		return;
+	}
+
+	float iconSize = fIcon->Bounds().Width();
+	DrawItemWithTextOffset(owner, frame, complete, iconSize + 4);
+
+	BRect iconFrame(frame.left + kLeftInset, frame.top,
+		frame.left + kLeftInset + iconSize - 1, frame.top + iconSize - 1);
+	owner->SetDrawingMode(B_OP_OVER);
+	owner->DrawBitmap(fIcon, iconFrame);
+	owner->SetDrawingMode(B_OP_COPY);
 }
 
 

src/preferences/locale/LanguageListView.h

@@ -20,10 +20,9 @@
 class LanguageListItem : public BStringItem {
 public:
 								LanguageListItem(const char* text,
-									const char* id, const char* langCode,
+									const char* id, const char* languageCode);
-									const char* countryCode = NULL);
+								LanguageListItem(
-								LanguageListItem(const LanguageListItem& other);
+									const LanguageListItem& other);
-	virtual						~LanguageListItem();
 
 			const BString&		ID() const { return fID; }
 			const BString&		Code() const { return fCode; }
@@ -31,11 +30,33 @@ public:
 	virtual	void				DrawItem(BView* owner, BRect frame,
 									bool complete = false);
 
-	virtual void				Update(BView* owner, const BFont* font);
+protected:
+			void				DrawItemWithTextOffset(BView* owner,
+									BRect frame, bool complete,
+									float textOffset);
 
 private:
 			BString				fID;
 			BString				fCode;
+};
+
+
+class LanguageListItemWithFlag : public LanguageListItem {
+public:
+								LanguageListItemWithFlag(const char* text,
+									const char* id, const char* languageCode,
+									const char* countryCode = NULL);
+								LanguageListItemWithFlag(
+									const LanguageListItemWithFlag& other);
+	virtual						~LanguageListItemWithFlag();
+
+	virtual void				Update(BView* owner, const BFont* font);
+
+	virtual	void				DrawItem(BView* owner, BRect frame,
+									bool complete = false);
+
+private:
+			BString				fCountryCode;
 			BBitmap*			fIcon;
 };
 

src/preferences/locale/LocaleWindow.cpp

@@ -98,7 +98,7 @@ LocaleWindow::LocaleWindow()
 	if (BLocaleRoster::Default()->GetAvailableLanguages(&availableLanguages)
 			== B_OK) {
 		BString currentID;
-		LanguageListItem* lastAddedCountryItem = NULL;
+		LanguageListItem* currentToplevelItem = NULL;
 
 		for (int i = 0; availableLanguages.FindString("language", i, &currentID)
 				== B_OK; i++) {
@@ -119,20 +119,23 @@ LocaleWindow::LocaleWindow()
 				}
 			}
 
-			LanguageListItem* item = new LanguageListItem(name,
+			LanguageListItem* item;
-				currentID.String(), currentLanguage.Code(),
+			if (currentLanguage.IsCountrySpecific()) {
-				currentLanguage.CountryCode());
+				item = new LanguageListItemWithFlag(name, currentID.String(),
-			if (currentLanguage.IsCountrySpecific()
+					currentLanguage.Code(), currentLanguage.CountryCode());
-				&& lastAddedCountryItem != NULL
-				&& lastAddedCountryItem->Code() == item->Code()) {
-				fLanguageListView->AddUnder(item, lastAddedCountryItem);
 			} else {
-				// This is a language variant, add it at top-level
+				item = new LanguageListItem(name, currentID.String(),
+					currentLanguage.Code());
+			}
+			if (currentLanguage.IsCountrySpecific()
+				&& currentToplevelItem != NULL
+				&& currentToplevelItem->Code() == item->Code()) {
+				fLanguageListView->AddUnder(item, currentToplevelItem);
+			} else {
+				// This is a generic language, add it at top-level
 				fLanguageListView->AddItem(item);
-				if (!currentLanguage.IsCountrySpecific()) {
+				item->SetExpanded(false);
-					item->SetExpanded(false);
+				currentToplevelItem = item;
-					lastAddedCountryItem = item;
-				}
 			}
 		}
 
@@ -181,38 +184,42 @@ LocaleWindow::LocaleWindow()
 		new BMessage(kMsgConventionsSelection));
 
 	// get all available formatting conventions (by language)
-	BFormattingConventions defaultConventions;
+	BFormattingConventions initialConventions;
-	BLocale::Default()->GetFormattingConventions(&defaultConventions);
+	BLocale::Default()->GetFormattingConventions(&initialConventions);
-	BString conventionID;
+	BString conventionsID;
 	fInitialConventionsItem = NULL;
-	LanguageListItem* lastAddedConventionsItem = NULL;
+	LanguageListItem* currentToplevelItem = NULL;
 	for (int i = 0;
-		availableLanguages.FindString("language", i, &conventionID) == B_OK;
+		availableLanguages.FindString("language", i, &conventionsID) == B_OK;
 		i++) {
-		BFormattingConventions convention(conventionID);
+		BFormattingConventions conventions(conventionsID);
-		BString conventionName;
+		BString conventionsName;
-		convention.GetName(conventionName);
+		conventions.GetName(conventionsName);
 
-		LanguageListItem* item = new LanguageListItem(conventionName,
+		LanguageListItem* item;
-			conventionID, convention.LanguageCode(), convention.CountryCode());
+		if (conventions.AreCountrySpecific()) {
-		if (!strcmp(conventionID, "en_US"))
+			item = new LanguageListItemWithFlag(conventionsName, conventionsID,
+				conventions.LanguageCode(), conventions.CountryCode());
+		} else {
+			item = new LanguageListItem(conventionsName, conventionsID,
+				conventions.LanguageCode());
+		}
+		if (!strcmp(conventionsID, "en_US"))
 			fDefaultConventionsItem = item;
-		if (conventionID.FindFirst('_') >= 0
+		if (conventions.AreCountrySpecific()
-			&& lastAddedConventionsItem != NULL
+			&& currentToplevelItem != NULL
-			&& lastAddedConventionsItem->Code() == item->Code()) {
+			&& currentToplevelItem->Code() == item->Code()) {
-			if (!strcmp(conventionID, defaultConventions.ID())) {
+			if (!strcmp(conventionsID, initialConventions.ID())) {
-				fConventionsListView->Expand(lastAddedConventionsItem);
+				fConventionsListView->Expand(currentToplevelItem);
 				fInitialConventionsItem = item;
 			}
-			fConventionsListView->AddUnder(item, lastAddedConventionsItem);
+			fConventionsListView->AddUnder(item, currentToplevelItem);
 		} else {
 			// This conventions-item isn't country-specific, add it at top-level
 			fConventionsListView->AddItem(item);
-			if (conventionID.FindFirst('_') < 0) {
+			item->SetExpanded(false);
-				item->SetExpanded(false);
+			currentToplevelItem = item;
-				lastAddedConventionsItem = item;
+			if (!strcmp(conventionsID, initialConventions.ID()))
-			}
-			if (!strcmp(conventionID, defaultConventions.ID()))
 				fInitialConventionsItem = item;
 		}
 	}
@@ -419,7 +426,7 @@ LocaleWindow::MessageReceived(BMessage* message)
 		{
 			MutableLocaleRoster::Default()->SetFilesystemTranslationPreferred(
 				fFilesystemTranslationCheckbox->Value());
-				
+
 			BAlert* alert = new BAlert(B_TRANSLATE("Locale"),
 				B_TRANSLATE("Deskbar and Tracker need to be restarted for this "
 				"change to take effect. Would you like to restart them now?"),
@@ -430,7 +437,7 @@ LocaleWindow::MessageReceived(BMessage* message)
 				NULL, be_app));
 			break;
 		}
-		
+
 		default:
 			BWindow::MessageReceived(message);
 			break;