Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bitmap font and TextField autoSize ignore lineHeight #1002

Open
soimy opened this issue Sep 12, 2017 · 6 comments
Open

Bitmap font and TextField autoSize ignore lineHeight #1002

soimy opened this issue Sep 12, 2017 · 6 comments

Comments

@soimy
Copy link

soimy commented Sep 12, 2017

There are issues reported lately:
soimy/msdf-bmfont-xml#8
feathersui/feathersui-starling#1633
When Textfield set to autoSize = VERTICAL, textField's height is cropped to bodyHeight and lineGap(shoulder) is ignored.
wx20170912-125419
In this case font.fnt metrics are:

  • fontSize: 42
  • lineHeight: 50
  • baseline: 32
  • lineGap: 8 (Equals lineHeight - fontSize)

Case 1: autoSize = VERTICAL

Smallest Textfield's height is lineHeight (50), smaller height will remove all text render.

Case 2: autoSize = BOTH_DIRECTION

Textfield's height is fontSize+1 (43)

Maybe it's better to make textfield's height consistent between these two cases.

@Maligan
Copy link

Maligan commented Sep 12, 2017

Ok, I for a long time this is not a bug, I think this is a feature... :-)
I write small UI toolkit and in my code I use extended TextField with next method to calculate textBounds:

/** This text bounds respect font lineHeight and it height can be only (lineHeight*scale * N). */
private function getTrueTextBounds(out:Rectangle = null):Rectangle
{
	out ||= new Rectangle();
	out.setEmpty();

	// for TrueType fonts
	var mesh:Mesh = getChildAt(0) as Mesh;
	var font:BitmapFont = getCompositor(format.font) as BitmapFont;
	if (font == null)
	{
		mesh.getBounds(this, out);
		out.inflate(-2, -2);
		return out;
	}

	// for empty text field
	var scale:Number = format.size / font.size;
	var numDrawableChars:int = mesh.numVertices / 4;
	if (numDrawableChars == 0)
	{
		out.setTo(0, 0, 0, font.lineHeight * scale);
		return out;
	}

	if (autoScale) scale = NaN;

	// find anchor chars

	var leftmostCharLeft:Number = NaN;
	var rightmostCharRight:Number = NaN;
	var bottommostCharTop:Number = NaN;
	var topmostCharTop:Number = NaN;

	var numNonDrawableChars:int = 0;

	for (var i:int = 0; i < text.length; i++)
	{
		var charID:int = text.charCodeAt(i);
		var char:BitmapChar = font.getChar(charID);
		var charQuadIndex:int = i - numNonDrawableChars;
		if (charQuadIndex >= numDrawableChars) break;

		var charIsDrawable:Boolean = char != null && (char.width != 0 && char.height != 0);
		if (charIsDrawable)
		{
			if (scale != scale)
			{
				var quadHeight:Number = mesh.getVertexPosition(charQuadIndex*4 + 3, _sPoint).y
									  - mesh.getVertexPosition(charQuadIndex*4 + 0, _sPoint).y;

				scale = quadHeight / char.height;
			}

			var charLeft:Number = mesh.getVertexPosition(charQuadIndex*4 + 0, _sPoint).x - char.xOffset*scale;
			if (charLeft < leftmostCharLeft || leftmostCharLeft != leftmostCharLeft)
				leftmostCharLeft = charLeft;

			var charRight:Number = mesh.getVertexPosition(charQuadIndex*4 + 1, _sPoint).x;
			if (charRight > rightmostCharRight || rightmostCharRight != rightmostCharRight)
				rightmostCharRight = charRight;

			var charTop:Number = _sPoint.y - char.yOffset*scale;
			if (charTop > bottommostCharTop || bottommostCharTop != bottommostCharTop)
				bottommostCharTop = charTop;

			if (charTop < topmostCharTop || topmostCharTop != topmostCharTop)
				topmostCharTop = charTop;
		}
		else
			numNonDrawableChars++;
	}

	// find width & height

	var lineHeight:Number = font.lineHeight*scale;
	var leading:Number = format.leading*scale;

	var numLines:int = 1 + int((bottommostCharTop-topmostCharTop) / (lineHeight+leading));
	var width:Number = rightmostCharRight - leftmostCharLeft;
	var height:Number = numLines*lineHeight + (numLines-1)*leading;

	out.setTo(leftmostCharLeft, topmostCharTop, width, height);
	return out;
}

This not give 100% good result there are some problems with different fonts without addition small changes.

But this method is dirty hack because TextField do not provides any text metrics such num lines etc.

PrimaryFeather added a commit that referenced this issue Sep 12, 2017
…height (refs #1002)

Instead, the last line now only verifies that there's enough room for the _font size_.
@PrimaryFeather
Copy link
Contributor

I just fixed that the text was not drawn at all if there was not enough space for the next lineHeight. Instead, I'm now checking if there's enough space for the font size. That's definitely how it's supposed to be done — thanks for making me aware of this!

As for the AutoSize changes, that's a little more tricky. Autosizing was more meant for "artistic" text, i.e. when you've got a score counter and want to have the smallest possible TextField.

Right now, auto-sizing is not even done by the ICompositor (the BitmapFont and TrueTypeCompositor classes), but by the TextField, calculated solely via the geometry found in the filled MeshBatches.

Instead, what you can always do is not use autoSize at all, but make use of the textBounds properties. After all, the actual size of a TextField doesn't make any difference on rendering — only for hitTest calculations.

@Maligan
Copy link

Maligan commented Sep 13, 2017

Yes, when I read autoSize related code I have feeling that this is like standalone mode, it makes sense.

One thing I do not like is "artistic" termin is not defined explicit, this generate various misunderstandings in text rendering.

As I write into msdf-bmfont issue, simple change autoSize from NONE to BOTH_DIRECTION gives unobvious result, on some fonts:

diff

After deep into typography text metrics I uderstand what can be mean under "artistic" - render font respect body height (from descender line to ascender line) ignore shoulders.
See this font metrics picture, which describe this terms.

Current autoSize implementation match this definition in case text string contains one char with ascender (like i, k, Ä etc. based on font) and one char with ascender (like g, j etc.).
But in other case autoSize give text height depends of text chars:

This may causes text dancing even on only number text field on fonts like Georgia.

TL;DR

I do not suggest change any Starling text render code.
I suggest feature request to msdf-bmfont: allow change offsetY of all chars by const value.
This change allow up all chars to erase font top shoulder fix situtation like on first pic.
This is analog of Starling BitmapFont offsetX, offsetY:

/** An offset that moves any generated text along the x-axis (in points).
 *  Useful to make up for incorrect font data. @default 0. */ 
public function get offsetX():Number { return _offsetX; }
public function set offsetX(value:Number):void { _offsetX = value; }

/** An offset that moves any generated text along the y-axis (in points).
 *  Useful to make up for incorrect font data. @default 0. */
public function get offsetY():Number { return _offsetY; }
public function set offsetY(value:Number):void { _offsetY = value; }

This feature allow prepare font asset correctly and avoid usage this values.

@PrimaryFeather
Copy link
Contributor

Yes, I understand the problem — and I agree with you that the current behaviour is not what some people might expect.

I've got a proposal: what if I add new TextFieldAutoSize constants, namely: BOTTOM and RIGHT, and maybe BOTTOM_RIGHT?

Those would leave the x and y values of the QuadBatch that displays the glyphs untouched, and would only change the width / height of the TextField so that it takes up just enough space as needed.

I'm aware that especially the term "right" is not ideal when talking about RTL-fonts. But since the Starling TextField doesn't do much in those terms, anyway ... 😬

@Maligan
Copy link

Maligan commented Sep 13, 2017

I do not think this is a good idea :-)

I fix all my problems use extended text field with overriden recompose method which respect font lineHeight when autoSize enabled, for me this is not a problem.

And again, I do not suggest any starling's change right now, I do not have much experience in it.


Daniel, you commit f67a204 have mistake, which break my code:

if (fontSize <= containerHeight)

This is not valid condition because containerHeight is "unscaled" available height, like _scale is unscaled fontSize, may be you mean

if (_size <= containerHeight)

@PrimaryFeather
Copy link
Contributor

Ah, damn — sorry for that! I just fixed that error. Thanks for making me aware of it!

I do not think this is a good idea :-)

Ah, okay, I see! Let me know if you change your mind — or, anyone else reading this, tell me if you want to see that happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants