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

Textbox: word widths not calculated using correct styles after line wrapping #3496

Closed
mlev opened this issue Dec 5, 2016 · 9 comments · Fixed by #3546
Closed

Textbox: word widths not calculated using correct styles after line wrapping #3496

mlev opened this issue Dec 5, 2016 · 9 comments · Fixed by #3546

Comments

@mlev
Copy link

mlev commented Dec 5, 2016

Version

1.7.1

Test Case

http://jsfiddle.net/mlev/j48qerLz/3/

Steps to reproduce

  • In textbox add 2 unbroken lines - with the second all bold (as per fiddle).
  • Inspect the dynamicMinWidth - this reflects the width of the bold text
  • Now add some spaces to the first line to make it wrap.
  • Again inspect the dynamicMinWidth - it will have changed even though the longest word hasn't.

Expected Behavior

The dynamicMinWidth should not change as the longest word hasn't changed.

Actual Behavior

The dynamicMinWidth reduces to the unstyled width of the longest word.

From what I can see - after the line wrapping occurs the style lookup fails to find the correct styles for the bold text. The style lookup in _getStyleDeclaration(lineIndex, charIndex) is done by line and char indexes - so I'm guessing this styles data does not match the line numbers of the wrapped text.

@asturur
Copy link
Member

asturur commented Dec 5, 2016

i guess we fixed this somehow.
Looks like not.

@mlev
Copy link
Author

mlev commented Dec 5, 2016

Yeah - doing a bit more debugging the lineIndex definitely seems off. i.e. _getWidthOfChar() (and subsequently _getStyleDeclaration()) is called with lineIndex=2 but the correct data for this line is stored in this.styles[1].

@asturur
Copy link
Member

asturur commented Dec 5, 2016

if instead of spaces you add some letter, does it behaves correctly?
Also the _getStyleDeclaration is a textbox or an itext method? textbox may need its own with stylemap

@felixroos
Copy link

I am not sure if this is related, but i also have an issue concerning textbox width and height calculation when using custom fonts. I made a fiddle based on the one provided by mlev:
http://jsfiddle.net/8bmhnLzx/
Here I am using the Google Font "Rogue Script" (The font is loaded as soon as you resize the texbox). The width and height of the Textbox are too small for its contents, cutting off parts of the text. Is there a way to just overflow the textbox or to give it extra padding in some way?

@asturur
Copy link
Member

asturur commented Dec 5, 2016

http://jsfiddle.net/8bmhnLzx/1/

us webfont loader with the active event to get font loaded immediately.
Fot the clipping part, the point is that fabric 1.7.0 use a cache system that add a canvas to host the object. Canvas api does not understand custom fonts dimensions see #1108 so for this particular case you should disable objectCaching.
read here:
http://fabricjs.com/fabric-object-caching

@mlev
Copy link
Author

mlev commented Dec 5, 2016

@asturur OK - so getting my head around this a bit more - I hadn’t noticed the TextBox vs IText methods.

What I’m seeing now is:

  1. I have 2 lines of text - the first one is wrapped the second one is a single long word. So visually there are 3 lines but only 1 line feed.
  2. When _measureText is called for the second line lineIndex=1
  3. When Textbox._getStyleDeclaration is called lineIndex=1
  4. After _styleMap lookup lineIndex=0, charIndex=12 - which refers to the wrapped word.
  5. Call to parent _getStyleDeclaration now returns nothing (or wrong style).

So looks like the styleMap lookup isn't working quite right.

@mlev
Copy link
Author

mlev commented Dec 6, 2016

Narrowed it down to the call to _wrapText in Textbox._splitTextIntoLines. Currently if the _styleMap is already populated it will use it - but if text already wrapped it will be out of sync because _wrapText uses the raw unwrapped text.

I've added a local fix in our code to patch the _splitTextIntoLines to set this._styleMap=null before the call _wrapText. The styleMap gets re-generated afterwards. Not sure if this is an suitable as a general fix though.

    _splitTextIntoLines: function() {
      var originalAlign = this.textAlign;
      this._styleMap = null;  <-- NEW LINE
      this.ctx.save();
      this._setTextStyles(this.ctx);
      this.textAlign = 'left';
      var lines = this._wrapText(this.ctx, this.text);
      this.textAlign = originalAlign;
      this.ctx.restore();
      this._textLines = lines;
      this._styleMap = this._generateStyleMap();
      return lines;
    },

@spc16670
Copy link

spc16670 commented Dec 6, 2016

+1

@asturur
Copy link
Member

asturur commented Dec 17, 2016

looks like good to me.

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

Successfully merging a pull request may close this issue.

4 participants