improved metrics for llfontgl::getWidth (use greater of character width/xadvance)

llfontgl::Addchar now called consistently when requesting font metrics
no longer possible to have font glyph info without rendered font

EXT-1294 - LLExpandableTextBox: wrong ellipses

reviewed by James and Mani

indra/llrender/llfontfreetype.cpp

@@ -98,9 +98,7 @@ LLFontGlyphInfo::LLFontGlyphInfo(U32 index)
 	mWidth(0),			// In pixels
 	mHeight(0),			// In pixels
 	mXAdvance(0.f),		// In pixels
-	mYAdvance(0.f),		// In pixels
-	mIsRendered(FALSE),
-	mMetricsValid(FALSE)
+	mYAdvance(0.f)		// In pixels
 {
 }
 
@@ -199,7 +197,7 @@ BOOL LLFontFreetype::loadFace(const std::string& filename, F32 point_size, F32 v
 	if (!mIsFallback)
 	{
 		// Add the default glyph
-		addGlyph(0, 0);
+		addGlyphFromFont(this, 0, 0);
 	}
 
 	mName = filename;
@@ -251,57 +249,10 @@ F32 LLFontFreetype::getXAdvance(llwchar wch) const
 	if (mFTFace == NULL)
 		return 0.0;
 
-	//llassert(!mIsFallback);
-	U32 glyph_index;
-
 	// Return existing info only if it is current
 	LLFontGlyphInfo* gi = getGlyphInfo(wch);
-	if (gi && gi->mMetricsValid)
-	{
-		return gi->mXAdvance;
-	}
-
-	const LLFontFreetype* fontp = this;
-	
-	// Initialize char to glyph map
-	glyph_index = FT_Get_Char_Index(mFTFace, wch);
-	if (glyph_index == 0)
-	{
-		font_vector_t::const_iterator iter;
-		for(iter = mFallbackFonts.begin(); (iter != mFallbackFonts.end()) && (glyph_index == 0); iter++)
-		{
-			glyph_index = FT_Get_Char_Index((*iter)->mFTFace, wch);
-			if(glyph_index)
-			{
-				fontp = *iter;
-			}
-		}
-	}
-	
-	if (glyph_index)
+	if (gi)
 	{
-		// This font has this glyph
-		fontp->renderGlyph(glyph_index);
-
-		// Create the entry if it's not there
-		char_glyph_info_map_t::iterator iter2 = mCharGlyphInfoMap.find(wch);
-		if (iter2 == mCharGlyphInfoMap.end())
-		{
-			gi = new LLFontGlyphInfo(glyph_index);
-			insertGlyphInfo(wch, gi);
-		}
-		else
-		{
-			gi = iter2->second;
-		}
-		
-		gi->mWidth = fontp->mFTFace->glyph->bitmap.width;
-		gi->mHeight = fontp->mFTFace->glyph->bitmap.rows;
-
-		// Convert these from 26.6 units to float pixels.
-		gi->mXAdvance = fontp->mFTFace->glyph->advance.x / 64.f;
-		gi->mYAdvance = fontp->mFTFace->glyph->advance.y / 64.f;
-		gi->mMetricsValid = TRUE;
 		return gi->mXAdvance;
 	}
 	else
@@ -323,10 +274,10 @@ F32 LLFontFreetype::getXKerning(llwchar char_left, llwchar char_right) const
 		return 0.0;
 
 	//llassert(!mIsFallback);
-	LLFontGlyphInfo* left_glyph_info = get_if_there(mCharGlyphInfoMap, char_left, (LLFontGlyphInfo*)NULL);
+	LLFontGlyphInfo* left_glyph_info = getGlyphInfo(char_left);;
 	U32 left_glyph = left_glyph_info ? left_glyph_info->mGlyphIndex : 0;
 	// Kern this puppy.
-	LLFontGlyphInfo* right_glyph_info = get_if_there(mCharGlyphInfoMap, char_right, (LLFontGlyphInfo*)NULL);
+	LLFontGlyphInfo* right_glyph_info = getGlyphInfo(char_right);
 	U32 right_glyph = right_glyph_info ? right_glyph_info->mGlyphIndex : 0;
 
 	FT_Vector  delta;
@@ -339,18 +290,10 @@ F32 LLFontFreetype::getXKerning(llwchar char_left, llwchar char_right) const
 BOOL LLFontFreetype::hasGlyph(llwchar wch) const
 {
 	llassert(!mIsFallback);
-	const LLFontGlyphInfo* gi = getGlyphInfo(wch);
-	if (gi && gi->mIsRendered)
-	{
-		return TRUE;
-	}
-	else
-	{
-		return FALSE;
-	}
+	return(mCharGlyphInfoMap.find(wch) != mCharGlyphInfoMap.end());
 }
 
-BOOL LLFontFreetype::addChar(llwchar wch) const
+LLFontGlyphInfo* LLFontFreetype::addGlyph(llwchar wch) const
 {
 	if (mFTFace == NULL)
 		return FALSE;
@@ -371,30 +314,23 @@ BOOL LLFontFreetype::addChar(llwchar wch) const
 			glyph_index = FT_Get_Char_Index((*iter)->mFTFace, wch);
 			if (glyph_index)
 			{
-				addGlyphFromFont(*iter, wch, glyph_index);
-				return TRUE;
+				return addGlyphFromFont(*iter, wch, glyph_index);
 			}
 		}
 	}
 	
 	char_glyph_info_map_t::iterator iter = mCharGlyphInfoMap.find(wch);
-	if (iter == mCharGlyphInfoMap.end() || !(iter->second->mIsRendered))
+	if (iter == mCharGlyphInfoMap.end())
 	{
-		BOOL result = addGlyph(wch, glyph_index);
-		return result;
+		return addGlyphFromFont(this, wch, glyph_index);
 	}
-	return FALSE;
-}
-
-BOOL LLFontFreetype::addGlyph(llwchar wch, U32 glyph_index) const
-{
-	return addGlyphFromFont(this, wch, glyph_index);
+	return NULL;
 }
 
-BOOL LLFontFreetype::addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch, U32 glyph_index) const
+LLFontGlyphInfo* LLFontFreetype::addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch, U32 glyph_index) const
 {
 	if (mFTFace == NULL)
-		return FALSE;
+		return NULL;
 
 	llassert(!mIsFallback);
 	fontp->renderGlyph(glyph_index);
@@ -417,8 +353,6 @@ BOOL LLFontFreetype::addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch,
 	// Convert these from 26.6 units to float pixels.
 	gi->mXAdvance = fontp->mFTFace->glyph->advance.x / 64.f;
 	gi->mYAdvance = fontp->mFTFace->glyph->advance.y / 64.f;
-	gi->mIsRendered = TRUE;
-	gi->mMetricsValid = TRUE;
 
 	insertGlyphInfo(wch, gi);
 
@@ -489,7 +423,11 @@ BOOL LLFontFreetype::addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch,
 		// omit it from the font-image.
 	}
 	
-	return TRUE;
+	LLImageGL *image_gl = mFontBitmapCachep->getImageGL(bitmap_num);
+	LLImageRaw *image_raw = mFontBitmapCachep->getImageRaw(bitmap_num);
+	image_gl->setSubImage(image_raw, 0, 0, image_gl->getWidth(), image_gl->getHeight());
+
+	return gi;
 }
 
 LLFontGlyphInfo* LLFontFreetype::getGlyphInfo(llwchar wch) const
@@ -499,7 +437,11 @@ LLFontGlyphInfo* LLFontFreetype::getGlyphInfo(llwchar wch) const
 	{
 		return iter->second;
 	}
-	return NULL;
+	else
+	{
+		// this glyph doesn't yet exist, so render it and return the result
+		return addGlyph(wch);
+	}
 }
 
 void LLFontFreetype::insertGlyphInfo(llwchar wch, LLFontGlyphInfo* gi) const
@@ -556,19 +498,12 @@ void LLFontFreetype::reset(F32 vert_dpi, F32 horz_dpi)
 
 void LLFontFreetype::resetBitmapCache()
 {
-	// Iterate through glyphs and clear the mIsRendered flag
-	for (char_glyph_info_map_t::iterator iter = mCharGlyphInfoMap.begin();
-		 iter != mCharGlyphInfoMap.end(); ++iter)
-	{
-		iter->second->mIsRendered = FALSE;
-		//FIXME: this is only strictly necessary when resetting the entire font, 
-		//not just flushing the bitmap
-		iter->second->mMetricsValid = FALSE;
-	}
+	for_each(mCharGlyphInfoMap.begin(), mCharGlyphInfoMap.end(), DeletePairedPointer());
+	mCharGlyphInfoMap.clear();
 	mFontBitmapCachep->reset();
 
 	// Add the empty glyph
-	addGlyph(0, 0);
+	addGlyphFromFont(this, 0, 0);
 }
 
 void LLFontFreetype::destroyGL()
@@ -576,21 +511,11 @@ void LLFontFreetype::destroyGL()
 	mFontBitmapCachep->destroyGL();
 }
 
-BOOL LLFontFreetype::getIsFallback() const
-{
-	return mIsFallback;
-}
-
 const std::string &LLFontFreetype::getName() const
 {
 	return mName;
 }
 
-F32 LLFontFreetype::getPointSize() const
-{
-	return mPointSize;
-}
-
 const LLPointer<LLFontBitmapCache> LLFontFreetype::getFontBitmapCache() const
 {
 	return mFontBitmapCachep;

indra/llrender/llfontfreetype.h

@@ -70,10 +70,8 @@ class LLFontGlyphInfo
 	S32 mHeight;		// In pixels
 	F32 mXAdvance;		// In pixels
 	F32 mYAdvance;		// In pixels
-	BOOL mMetricsValid; // We have up-to-date metrics for this glyph
 
 	// Information for actually rendering
-	BOOL mIsRendered;	// We actually have rendered this glyph
 	S32 mXBitmapOffset; // Offset to the origin in the bitmap
 	S32 mYBitmapOffset; // Offset to the origin in the bitmap
 	S32 mXBearing;	// Distance from baseline to left in pixels
@@ -133,34 +131,27 @@ class LLFontFreetype : public LLRefCount
 	F32 getXAdvance(llwchar wc) const;
 	F32 getXKerning(llwchar char_left, llwchar char_right) const; // Get the kerning between the two characters
 
-	BOOL hasGlyph(llwchar wch) const;		// Has a glyph for this character
-	BOOL addChar(llwchar wch) const;		// Add a new character to the font if necessary
-	BOOL addGlyph(llwchar wch, U32 glyph_index) const;	// Add a new glyph to the existing font
-	BOOL addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch, U32 glyph_index) const;	// Add a glyph from this font to the other (returns the glyph_index, 0 if not found)
-
 	LLFontGlyphInfo* getGlyphInfo(llwchar wch) const;
 
-	void insertGlyphInfo(llwchar wch, LLFontGlyphInfo* gi) const;
-	void renderGlyph(U32 glyph_index) const;
-
 	void reset(F32 vert_dpi, F32 horz_dpi);
-	void resetBitmapCache();
 
 	void destroyGL();
 
-	BOOL getIsFallback() const;
-
 	const std::string& getName() const;
 
-	F32 getPointSize() const;
-
 	const LLPointer<LLFontBitmapCache> getFontBitmapCache() const;
 
 	void setStyle(U8 style);
 	U8 getStyle() const;
 
 private:
+	void resetBitmapCache();
 	void setSubImageLuminanceAlpha(U32 x, U32 y, U32 bitmap_num, U32 width, U32 height, U8 *data, S32 stride = 0) const;
+	BOOL hasGlyph(llwchar wch) const;		// Has a glyph for this character
+	LLFontGlyphInfo* addGlyph(llwchar wch) const;		// Add a new character to the font if necessary
+	LLFontGlyphInfo* addGlyphFromFont(const LLFontFreetype *fontp, llwchar wch, U32 glyph_index) const;	// Add a glyph from this font to the other (returns the glyph_index, 0 if not found)
+	void renderGlyph(U32 glyph_index) const;
+	void insertGlyphInfo(llwchar wch, LLFontGlyphInfo* gi) const;
 
 	std::string mName;
 

indra/llrender/llfontgl.cpp

@@ -252,11 +252,6 @@ S32 LLFontGL::render(const LLWString &wstr, S32 begin_offset, F32 x, F32 y, cons
 	{
 		llwchar wch = wstr[i];
 
-		if (!mFontFreetype->hasGlyph(wch))
-		{
-			addChar(wch);
-		}
-
 		const LLFontGlyphInfo* fgi= mFontFreetype->getGlyphInfo(wch);
 		if (!fgi)
 		{
@@ -299,10 +294,6 @@ S32 LLFontGL::render(const LLWString &wstr, S32 begin_offset, F32 x, F32 y, cons
 		if (next_char && (next_char < LAST_CHARACTER))
 		{
 			// Kern this puppy.
-			if (!mFontFreetype->hasGlyph(next_char))
-			{
-				addChar(next_char);
-			}
 			cur_x += mFontFreetype->getXKerning(wch, next_char);
 		}
 
@@ -442,15 +433,22 @@ F32 LLFontGL::getWidthF32(const llwchar* wchars, S32 begin_offset, S32 max_chars
 
 	F32 cur_x = 0;
 	const S32 max_index = begin_offset + max_chars;
-	for (S32 i = begin_offset; i < max_index; i++)
+
+	F32 width_padding = 0.f;
+	for (S32 i = begin_offset; i < max_index && wchars[i] != 0; i++)
 	{
 		llwchar wch = wchars[i];
-		if (wch == 0)
-		{
-			break; // done
-		}
 
-		cur_x += mFontFreetype->getXAdvance(wch);
+		const LLFontGlyphInfo* fgi= mFontFreetype->getGlyphInfo(wch);
+
+		F32 advance = mFontFreetype->getXAdvance(wch);
+
+		// for the last character we want to measure the greater of its width and xadvance values
+		// so keep track of the difference between these values for the each character we measure
+		// so we can fix things up at the end
+		width_padding = llmax(0.f, (F32)fgi->mWidth - advance);
+
+		cur_x += advance;
 		llwchar next_char = wchars[i+1];
 
 		if (((i + 1) < begin_offset + max_chars) 
@@ -464,6 +462,9 @@ F32 LLFontGL::getWidthF32(const llwchar* wchars, S32 begin_offset, S32 max_chars
 		cur_x = (F32)llfloor(cur_x + 0.5f);
 	}
 
+	// add in extra pixels for last character's width past its xadvance
+	cur_x += width_padding;
+
 	return cur_x / sScaleX;
 }
 
@@ -663,25 +664,6 @@ S32 LLFontGL::charFromPixelOffset(const llwchar* wchars, S32 begin_offset, F32 t
 	return llmin(max_chars, pos - begin_offset);
 }
 
-BOOL LLFontGL::addChar(llwchar wch) const
-{
-	if (!mFontFreetype->addChar(wch))
-	{
-		return FALSE;
-	}
-
-	stop_glerror();
-
-	LLFontGlyphInfo *glyph_info = mFontFreetype->getGlyphInfo(wch);
-	U32 bitmap_num = glyph_info->mBitmapNum;
-
-	const LLFontBitmapCache* font_bitmap_cache = mFontFreetype->getFontBitmapCache();
-	LLImageGL *image_gl = font_bitmap_cache->getImageGL(bitmap_num);
-	LLImageRaw *image_raw = font_bitmap_cache->getImageRaw(bitmap_num);
-	image_gl->setSubImage(image_raw, 0, 0, image_gl->getWidth(), image_gl->getHeight());
-	return TRUE;
-}
-
 const LLFontDescriptor& LLFontGL::getFontDesc() const
 {
 	return mFontDescriptor;

indra/llrender/llfontgl.h

@@ -131,8 +131,6 @@ class LLFontGL
 	// Returns the index of the character closest to pixel position x (ignoring text to the right of max_pixels and max_chars)
 	S32 charFromPixelOffset(const llwchar* wchars, S32 char_offset, F32 x, F32 max_pixels=F32_MAX, S32 max_chars = S32_MAX, BOOL round = TRUE) const;
 
-	BOOL addChar(const llwchar wch) const;
-
 	const LLFontDescriptor& getFontDesc() const;
 
 

indra/llui/lltextbase.cpp

@@ -1106,12 +1106,17 @@ void LLTextBase::reflow(S32 start_index)
 
 			S32 segment_width = segment->getWidth(seg_offset, character_count);
 			remaining_pixels -= segment_width;
-			S32 text_left = getLeftOffset(text_width - remaining_pixels);
 
 			seg_offset += character_count;
 
 			S32 last_segment_char_on_line = segment->getStart() + seg_offset;
 
+			S32 text_left = getLeftOffset(text_width - remaining_pixels);
+			LLRect line_rect(text_left, 
+							cur_top, 
+							text_left + (text_width - remaining_pixels), 
+							cur_top - line_height);
+
 			// if we didn't finish the current segment...
 			if (last_segment_char_on_line < segment->getEnd())
 			{
@@ -1129,10 +1134,7 @@ void LLTextBase::reflow(S32 start_index)
 				mLineInfoList.push_back(line_info(
 											line_start_index, 
 											last_segment_char_on_line, 
-											LLRect(text_left, 
-													cur_top, 
-													text_left + (text_width - remaining_pixels),
-													cur_top - line_height), 
+											line_rect, 
 											line_count));
 
 				line_start_index = segment->getStart() + seg_offset;
@@ -1147,15 +1149,12 @@ void LLTextBase::reflow(S32 start_index)
 				mLineInfoList.push_back(line_info(
 											line_start_index, 
 											last_segment_char_on_line, 
-											LLRect(text_left, 
-													cur_top, 
-													text_left + (text_width - remaining_pixels),
-													cur_top - line_height), 
+											line_rect, 
 											line_count));
 				cur_top -= llround((F32)line_height * mLineSpacingMult) + mLineSpacingPixels;
 				break;
 			}
-			// finished a segment and there are segments remaining on this line
+			// ...or finished a segment and there are segments remaining on this line
 			else
 			{
 				// subtract pixels used and increment segment

indra/newview/llexpandabletextbox.cpp

@@ -57,7 +57,20 @@ class LLExpanderSegment : public LLTextSegment
 	{ 
 		return start_offset;
 	}
-	/*virtual*/ S32		getNumChars(S32 num_pixels, S32 segment_offset, S32 line_offset, S32 max_chars) const { return getEnd() - getStart(); }
+	/*virtual*/ S32		getNumChars(S32 num_pixels, S32 segment_offset, S32 line_offset, S32 max_chars) const 
+	{ 
+		// require full line to ourselves
+		if (line_offset == 0) 
+		{
+			// print all our text
+			return getEnd() - getStart(); 
+		}
+		else
+		{
+			// wait for next line
+			return 0;
+		}
+	}
 	/*virtual*/ F32		draw(S32 start, S32 end, S32 selection_start, S32 selection_end, const LLRect& draw_rect)
 	{
 		F32 right_x;

indra/newview/skins/default/xui/en/widgets/expandable_text.xml

@@ -5,6 +5,7 @@
    more_label="More" 
   follows="left|top"
   name="text"
+   allow_scroll="true" 
   use_ellipses="true"
   word_wrap="true"
   tab_stop="true"