openFrameworks icon indicating copy to clipboard operation
openFrameworks copied to clipboard

fix ofTrueTypeFont::stringWidth

Open dimitre opened this issue 3 years ago • 5 comments

I'm submitting this PR to start a discussion. stringWidth actually get width of getStringBoundingBox, which uses a mixture of glyphProperties advance and xmin+width, that are not the same thing.

this PR fixes getStringBoundingBox, makes stringWidth independent of the function, so it is less overhead.

address the issue discussed here https://forum.openframeworks.cc/t/are-monospace-fonts-really-monospace/40358/3

dimitre avatar Sep 03 '22 17:09 dimitre

Amazing!!! I’ve discovered this bug only recently and it makes so much sense seeing this fix!

I’ll test the changes as well

On Mon, 5 Sep 2022 at 6:34 am, Dimitre @.***> wrote:

@.**** commented on this pull request.

In libs/openFrameworks/graphics/ofTrueTypeFont.cpp https://github.com/openframeworks/openFrameworks/pull/7065#discussion_r962366040 :

@@ -1162,8 +1164,16 @@ void ofTrueTypeFont::drawCharAsShape(uint32_t c, float x, float y, bool vFlipped

//----------------------------------------------------------- float ofTrueTypeFont::stringWidth(const std::string& c) const{

  • ofRectangle rect = getStringBoundingBox(c, 0,0);
  • return rect.width; +// return getStringBoundingBox(c, 0,0).width;
  • int w = 0;
  • iterateString( c, 0, 0, false, [&]( uint32_t c, glm::vec2 pos ){
  • if ( c == '\t' ){
    
  • 	w += getGlyphProperties(' ').advance * spaceSize * TAB_WIDTH;
    
  • } else {
    
  • 	w += getGlyphProperties( c ).advance;
    
  • }
    
  • });
  • return w;

Yes I think the same. I'll take some time to test with right to left texts too, just to see if it works correctly

— Reply to this email directly, view it on GitHub https://github.com/openframeworks/openFrameworks/pull/7065#discussion_r962366040, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGK2HCZKKT3SGRIVCAHKTDV4UBU3ANCNFSM6AAAAAAQD67CDM . You are receiving this because you are subscribed to this thread.Message ID: @.***>

danoli3 avatar Sep 05 '22 01:09 danoli3

I've just tested and both this PR update and the original code doesn't return any width for right to left text. so I think at least we don't have any regression

	cout << font.stringWidth("برامج") << endl;

dimitre avatar Sep 05 '22 02:09 dimitre

Some new changes address this issue from 2016 https://github.com/openframeworks/openFrameworks/issues/5138

I think somebody used long variables (integer) to store 26.6 fixed-point data directly from Freetype, but instead stored bit shifting and converting but storing in long, so metrics are losing precision.

I've used functions to convert back and forth this kind of number to double or float, so things get much more precise.

dimitre avatar Sep 05 '22 04:09 dimitre

Oh amazing! I’ll check this out see if solves all my glyph width issues

On Mon, 5 Sep 2022 at 2:11 pm, Dimitre @.***> wrote:

Some new changes address this issue from 2016 #5138 https://github.com/openframeworks/openFrameworks/issues/5138

I think somebody used long variables (integer) to store 26.6 fixed-point data directly from Freetype, but instead stored bit shifting and converting but storing in long, so metrics are losing precision.

I've used functions to convert back and forth this kind of number to double or float, so things get much more precise.

— Reply to this email directly, view it on GitHub https://github.com/openframeworks/openFrameworks/pull/7065#issuecomment-1236522951, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGK2HEEVBZKABZ7GUFWV6LV4VXIJANCNFSM6AAAAAAQD67CDM . You are receiving this because you commented.Message ID: @.***>

danoli3 avatar Sep 05 '22 05:09 danoli3

@ofTheo here I used an example of a font with a negative xMin in relation to origin. It is Zapfino font present on macOs, but as we can see in this software the character width only consider the distance from origin to character advance.

We can see a bounding box there too disregarding font bleed to the left and right, I think we can use only advance to measure string and bounding box. It is simpler and 99% of the time we won't use this kind of font.

If we consider xmin and xmax we have to consider if the character is the first or last in the string, as the other metrics we can know all from advance which considers kerning too.

Screen Shot 2022-09-20 at 21 30 25

dimitre avatar Sep 21 '22 00:09 dimitre

thanks for the example @dimitre I think your approach sounds good as long as you think it won't break people's current usage in a big way.

we could also add additional functions for other metrics like bounding box with font bleed etc.

ofTheo avatar Sep 28 '22 16:09 ofTheo

Yes I think it can be merged as it is since it fixes long time integer issues, path simplification artifacts And then when we have time we can improve functions, consider right to left texts, etc.

dimitre avatar Sep 28 '22 16:09 dimitre

closes https://github.com/openframeworks/openFrameworks/issues/6592

dimitre avatar Sep 28 '22 16:09 dimitre