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

It would be great if konva would let us control ctx.textBaseline for Text #559

Closed
kickthemooon opened this issue Jan 27, 2019 · 11 comments
Closed

Comments

@kickthemooon
Copy link

As far as I can understand from the code the canvas textBaseline is hard coded as "MIDDLE".

Would it be hard to make that configurable?

In my case I would need the default textBaseline "alphabetic" because alphabetic seems to be equal to the actual font baseline and I need to do some calculations relative to the font baseline.

@lavrton
Copy link
Member

lavrton commented Jan 28, 2019

@kickthemooon what is your use case? Why exactly do you need it?

@kickthemooon
Copy link
Author

kickthemooon commented Jan 29, 2019

The use case is that Canvas renders some fonts inproperly placing them to high in the Canvas so the font gets cut off at the top.

Text baseline "alphabetic" (default) is the only textBaseline option that I could correlate to an actual font metric which is the font baseline.

When alphabetic is used It is possible to caculate Y for the canvas text by having the font ascent to baseline ratio.

https://jsfiddle.net/n1kLvro0/2/

That way the font is drawn on the canvas like it is rendered in an html element.

@lavrton
Copy link
Member

lavrton commented Jan 31, 2019

Hmm. I am not sure how we will handle hit detection and getClientRect() calculations in case of custom baseline.

@lavrton
Copy link
Member

lavrton commented Feb 25, 2019

I am closing the issue because I am not sure what exactly should we fix from Konva side. Making baseline configurable will make the internal code more complex.

But I am happy to see Pull Requests for some fixes, or direct demos with Konva where current default options don't work properly.

@lavrton lavrton closed this as completed Feb 25, 2019
@rxsto
Copy link

rxsto commented Feb 16, 2023

Hey there @lavrton! You've got some great stuff going on with Konva, enjoying it very much.

I just recently came across this issue where I personally was quite sure that Konva did render fonts correctly. We've been having issues with equalizing rendering for both frontend and backend, so I naturally put the blame on our immature backend libraries.
Upon further digging this same issue came across my mind: when using non-Latin characters, a 0,0 positioned text element will cut of certain parts of the content (for context, this issue seems related too). I was curious about how Konva actually calculated width and especially height of the text node, and the calculation used here just seemed to make absolutely no sense.
After experimenting with certain values, measuring pixels and reading into modern kerning using GPOS (which thankfully is already support as a default in canvas) I started using ascender and descender values for calculating the actual height of the node, and voila, the text was being displayed properly!

The changes I made are public in this fork: https://github.com/bot-hydra/konva

I didn't open up a pull request (yet) since I am not too sure about this solution being production-ready. Feel free to check out what I'm proposing and potentially use it as a hint for a proper implementation. For now I am hosting the modified module on GitHub packages, but would love to switch back once Konva actually supports correct text positioning!

@lavrton
Copy link
Member

lavrton commented Feb 20, 2023

@rxsto It will be really nice if you can make a pull request and especially good if you can make any tests for that change. Probably, we can compare rendering with DOM text.

@MykhailoIskiv
Copy link

I was able to fix the issue in my project.
The difference with text rendering on canvas and in HTML is very noticeable when there is a big difference between height of font Ascent and height of font Descent.
To to fix it we need to find difference between them and divide it by 2.
so offset would be
offset = (fontBoundingBoxAscent - fontBoundingBoxDescent) / 2

use measureText to get fontBoundingBoxAscent and fontBoundingBoxDescent;
https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/measureText
Make sure that same font family/style/size... is applied as as you applied to Konva.Text before calling measureText;
I also set 'contex.textBaseline = 'middle' to have it the same as in Konva.Text, just in case
Once we have offset, we apply it to Konva.Text y position.
After the above changes I was able to make font rendering the same in Konva.Text and HTML

I'm not very familiar with the library internals, but probably following changes should be made

measureSize function in Konva.Text should be extended to return fontBoundingBoxAscent and fontBoundingBoxDescent
https://github.com/konvajs/konva/blob/master/src/shapes/Text.ts#L391

and then, probably in _sceneFunc,
offset (fontBoundingBoxAscent - fontBoundingBoxDescent) / 2 should be added to _partialTextY
https://github.com/konvajs/konva/blob/master/src/shapes/Text.ts#L327

@lavrton lavrton reopened this May 16, 2024
@lavrton
Copy link
Member

lavrton commented May 16, 2024

I need a help with testing. Ideas from @rxsto and @MykhailoIskiv gave me some thoughts to try the similar approach.
I am asking for a massive test.
I added a new property for a text element textBaseline. It can only be middle (current rendering logic) or alphabetic (for updated render logic with adjusted y position).
I wish to make the new logic the default one, but I need to make sure it doesn't break things.

To enable the fix:

  1. Upgrade Konva to version 9.3.8
  2. Set Konva._fixTextRendering = true

@lavrton
Copy link
Member

lavrton commented May 16, 2024

Updated instructions on how to enable that mode until it is the default one.

@Adam-Greenan
Copy link

We have implemented this fix, and will do ongoing tests with our staging website and get back to you with results.

Thanks for getting around to this issue, glad to see a fix in place!

@lavrton
Copy link
Member

lavrton commented Jun 12, 2024

Closing the issue for having Konva._fixTextRendering = true as a solution. I want to make it a default behavior one day. But I need a lot more feedback.

@lavrton lavrton closed this as completed Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants