Font should call isSuitableForFont() on height change

General JUCE discussion
1

Font::setHeight() needs to call isSuitableForFont() on the typface, passing the new Font object, and clear the old Typeface if it is not suitable instead of just incrementing the reference count.

This fixes a problem where calling setHeight() on a Font using a hinted face will use the old set of glyphs.

There’s some discussion here:

http://www.rawmaterialsoftware.com/viewtopic.php?f=2&t=9912&start=15

Looking over the Font code, I think that anywhere dupeInternalIfShared() we might need to also call isSuitableForFont(). This affects many “set” functions.

I don’t fully understand the pimpl used in Font, but this might be as easy as adding this line at the end of each function that calls dupeInternalIfShared:

if (font->typeface && !font->typeface->isSuitableForFont (*this))
  font->typeface = nullptr;
2

im thinking it could be fixed in `Font::getTypeface’

something like:

NOT TESTED!

[code]Typeface* Font::getTypeface() const
{
if (font->typeface == nullptr || !font->typeface->isSuitableForFont(*this))
font->typeface = TypefaceCache::getInstance()->findTypefaceFor (*this);

return font->typeface;

}
[/code]

3

Hmmm…I’m not fond of that change, because it allows the Font object to be left in an inconsistent state for a little while (the Typeface won’t match the specs).

4

That’s true, but pretty much everything calls getTypface before use it. Also, in a way you want it to delay the creation of the typeface as much as possible. for example, if not used or the spec changes before it’s used. This is more important for hinted sizes where each size is a new set of glyphs.

However, either way is good. compared to my workaround.

5

My opinion, Font shouldn’t even have members like setHeight(). It’s already a lightweight copyable object that holds a pimpl, to change the height might as well just make a new Font object. Font should be immutable.

6

I agree, and there’s a withHeight method that I use nowadays. Unfortunately, getting rid of setHeight would break a lot of other people’s code, so it’ll have to stay for a while.

I’ve not had chance to read through all your font-related threads… is there a TL;DR for it all?

7

Yes:

[quote]
I don’t fully understand the pimpl used in Font, but this might be as easy as adding this line at the end of each function that calls dupeInternalIfShared:

if (font->typeface && !font->typeface->isSuitableForFont (*this)) font->typeface = nullptr; [/quote]

8

Ta.