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

removing some serious inefficiencies, fixing a change that broke compatibility with opengl < 3 #44

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

slowriot
Copy link

See commit descriptions for more details.

…ch is then reversed by a custom transform matrix. What were you thinking??

Reverting to using the identity matrix (although technically it isn't necessary to set this explicitly at all since it's used by default)
…ont_load_glyphs() only to immediately load it again to calculate the kerning.

Explanation: texture_font_generate_kerning() is called only in one place in the entire library, and that's inside texture_font_load_glyphs().  Moving it to before the cleanup of library and face allows us to avoid a costly and completely pointless unload followed by reload.  texture_font_generate_kerning() is not declared in the headers so there is no risk of it being called elsewhere, so this change has no risk of breaking anything in any existing code.  A pointer to the existing face is passed along instead.
…GL_RED

Older versions of OpenGL do not support GL_RED but newer ones still do support GL_ALPHA.  Rather than switching the whole thing to a deprecated format (which requires changes to the shaders also) for the sake of compatibility, I'm instead adding an #ifdef switch.
@rougier
Copy link
Owner

rougier commented Sep 18, 2014

Could you make a new PR ?

…ating its define from that of horizontal resolution to avoid confusion and increase configurability
@slowriot
Copy link
Author

Have added a new commit re-adding the stretched matrix (in a tidier form) and added a control to toggle this behaviour on or off, as well as adding your links to the documentation for that control.

@rougier
Copy link
Owner

rougier commented Sep 18, 2014

Thanks. I won"t be avail for one week (no internet). I will test & merge once I'm back.
Don't hesitate to ping me

@behdad
Copy link

behdad commented Sep 18, 2014

To disable horizontal hinting you can use hinting mode slight.

@rougier
Copy link
Owner

rougier commented Oct 2, 2014

@behdad you mean directly from freetype ? I get only FT_LOAD_NO_HINTING
@slowriot I will test soon.

@behdad
Copy link

behdad commented Oct 2, 2014

@rougier Check FT_LOAD_TARGET_LIGHT

@rougier
Copy link
Owner

rougier commented Oct 2, 2014

Not quite sure this would be equivalent to horizontal hinting discard:

  • FT_RENDER_MODE_NORMAL This is the default render mode; it corresponds to 8-bit anti-aliased bitmaps.
  • FT_RENDER_MODE_LIGHT This is equivalent to FT_RENDER_MODE_NORMAL. It is only defined as a separate value because render modes are also used indirectly to define hinting algorithm selectors. See FT_LOAD_TARGET_XXX for details.

@behdad
Copy link

behdad commented Oct 2, 2014

   *   FT_LOAD_TARGET_LIGHT :: 
   *     A lighter hinting algorithm for non-monochrome modes.  Many 
   *     generated glyphs are more fuzzy but better resemble its original 
   *     shape.  A bit like rendering on Mac OS~X. 

Also, from aflatin.c:

    /* 
     *  In `light' hinting mode we disable horizontal hinting completely. 
     *  We also do it if the face is italic. 
     */ 
    if ( mode == FT_RENDER_MODE_LIGHT                      || 
         ( face->style_flags & FT_STYLE_FLAG_ITALIC ) != 0 ) 
      scaler_flags |= AF_SCALER_FLAG_NO_HORIZONTAL; 

@rougier
Copy link
Owner

rougier commented Oct 3, 2014

@behdad Thanks.
@slowriot Might be a better solution, what do you think ?

@slowriot
Copy link
Author

slowriot commented Oct 3, 2014

@rougier I personally think exposing as much of the underlying Freetype functionality to the user as possible is the best approach, so the user can select the right hinting mode for their application.

I also think the default behaviour should always be in accordance with the "principle of least surprise", with the user requesting any unexpected or special hinting modes specifically. In the case of the existing hack to suppress horizontal hinting, I personally was quite surprised when I came across it, so I think defaulting to a more standard mode would be better along with adding some mechanism of allowing the user to select the hinting mode they want. And documenting the font hinting behaviour much better! :)

@behdad
Copy link

behdad commented Oct 3, 2014

@slowriot You are right in principle. But horizontal hinting doesn't make sense in this case because the SDF is supposed to be size-independent.

@slowriot
Copy link
Author

slowriot commented Oct 3, 2014

@behdad Freetype-gl isn't used exclusively with SDF rendering, and moreover why would you want to enable vertical hinting but not horizontal in that case? Either way the result is potentially surprising and unexpected to the user. Best case is that they don't notice anything is wrong; worst case is that text renders incorrectly without the user being given the option to switch behaviour.

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

Successfully merging this pull request may close these issues.

3 participants