Deprecation and JUCE; and also font width

Development
1

Hi

I’m going to not move Surge to JUCE 8 this morning because not only is juce::Font deprecated (which i had worked around when I went to 8.0.1) but in 8.0.2 you also deprecated juce::Font::getFontWidth

So I have two types of comments

First: Is it really appropriate to deprecate public apis on a point release? Obviously my question implies that I don’t think it is but wondering what’s your strategy here. Since moving our submodule isn’t that easy (we have a couple of patches to work in some non standard linux configurations) I went to 802 rather than 801 and then found hey I have another 300 changes to make. I sort of think “this API becomes deprecated” is a 8.x.0 not an 8.x.y change

Second: The comment on the deprecated function is this

        If you are trying to find the amount of space required to display a given string,
        you'll get more accurate results by actually measuring the results of whichever
        text layout engine (e.g. GlyphArrangement, TextLayout) you'll use when displaying
        the string.

some thoughts on this

  1. Is the accuracy difference enough to deprecate the function?
  2. How do I know which text layout engine I’m using? I just use g.drawText. Perhaps expanding the comment so the user isn’t completely flummoxed by the choice you present would be handy.

But also, supporting a big bit of software which builds in lots and lots of environments requires me to run with warnings and errors on in many cases. Deprecation warnings are real and expensive. And deprecation is all-or-nothing the way compilers work (and the way you choose to deprecate. You could I suppose introduce a JUCE_SUPRESS_FONT_WIDTH_DEPRECATION option and ifdef your code so we can partial suppress the warning).

So my real question is what’s my expectation as a user on when you will deprecate things inside a third-dot release. but hope the other comments are useful also.

Thanks as always

6 Likes
2

By not supporting any Long Term version, and clearly ditching / killing all support for Juce 7, the Juce staff (and Pace as owning company) lost my respect for how they operate.

Rewriting the rendering engine for Windows and effectively frustrating users who want to keep using the previous engine is also a bad and unfriendly practise in my opinion.

When the licensing changes were announced for Juce 8, I dediced to switch to the longer term Juce 7 license, which is ok for my plugins, but a bit later came the surprise that version 7 will not even get hotfixes if needed, and this week I read that by doing fixes yourself you risk infringing with Juce “IP” in version 8 commits. So you apparently need a clean room approach for fixing bugs in Juce 7…

Not happy with how Juce and Pace handle these transitions. With the money that I pay to Pace, I would expect to be able to use any version of Juce, without licensing costs and these silly changing rules. But that’s another topic.

3

For future readers here’s a little per script i used to change my code from blah.getStringWidth to juce::Blah::getStringWidth(blah

#!/usr/bin/perl


use File::Find;
use File::Basename;

find(
    {
        wanted => \&findfiles,
    },
    'src'
);

sub findfiles
{
    $f = $File::Find::name;
    print $f . "\n";
    if ($f =~ m/\.cpp$/ || $f =~ m/\.h$/)
    {
        $q = basename($f);

        open (IN, "< $q") || die "Cant open $q from $f";
        open (OUT, "> $q.bak");
        while(<IN>)
        {
            if (m/getStringWidth/)
            {
                print;
                s/([^\s\{]+).getStringWidth\(/(int)juce::GlyphArrangement::getStringWidth\($1, /;
                print;
                #die;
            }
            print OUT;

        }
        close(IN);
        close(OUT);
        system("mv ${q}.bak ${q}");
    }
}
5

@baconpaul thanks for the feedback.

I think the issue comes about when fallback fonts come into play, it’s probably a bad idea to be asking a single font for a string width because it depends what fonts actually get picked.

For g.drawText() use GlyphArrangement, TextLayout is for when you’re using an AttributedString so the text may itself be using different fonts at different points within the text.

To be clear are you saying if the version was tagged as 8.1 you would have been OK with the change? Historically I don’t think JUCE has traditionally adhered to semantic versioning.

6

I think yeah if there had been a new set of deprecations at 8.1 then that would be fine. 8.0.2 seems like ‘oh here’s the bugs fixed in 8.0.1’ not ‘and change your string width calls in 94 places’. I figure the other fixes in 01-02 are worthwhile but code breaking changes make it hard to absorb them.

I think adding your comment about GlyphArrangement to the comment in the function would be super useful.

With my perl script I got surge converted and just merged the move to 8.0.2.

Thanks as always

1 Like
7

I’d point out that the JUCE Class Reference Docs do not (yet) have anything about the deprecation of Font::getStringWidth(), and it would seem this would be a good place to include:

If you are trying to find the amount of space required to display a given string,
       you'll get more accurate results by actually measuring the results of whichever
       text layout engine (e.g. GlyphArrangement, TextLayout) you'll use when displaying
       the string.

and…

For `g.drawText()` use `GlyphArrangement`, `TextLayout` is for when you’re using an `AttributedString` so the text may itself be using different fonts at different points within the text.
8

@stephenk I’m not sure what you mean the JUCE docs have that first quote here.

1 Like
9

@anthony-nicholls I apologize, somehow I was apparently looking at a cached version on my computer; I reloaded and now I see it. I do like the idea of adding more info about how to decide which version to use, though. :wink:

2 Likes
10

I don’t really get the distinction. In old API we asked a Font its width, in the new API we have a Font and we ask for its width.

I looked at the juce changes here: Font: Deprecate getStringWidth and getGlyphPositions · juce-framework/JUCE@29213e0 · GitHub

Every single one replaces font.getStringWidth (text) with GlyphArrangement::getStringWidth (font, text)

If A can just be mindlessly replaced with B, why not just make Font::getStringWidth call GlyphArrangement::getStringWidth and then everybody gets new functionality and nothing needs to be deprecated?

2 Likes
11

I’m inclined to agree - I don’t understand the need to deprecate things that have drop-in replacements.

The default Font{} constructor is another example - why did I need to go around replacing Font{} with Font{FontOptions{}}, couldn’t that have just been the new default implementation? And even the non-default constructors for that matter, couldn’t they have just forwarded to font options?

4 Likes
12

I almost did that in my fork yeah but we have some folks who vendor or devendor (whichever one means “don’t use our deps and hope it works anyway and send us support tickets when it doesn’t”) juce so I wrote the Perl script

One note. Glyph get width returns a float so the appropriate replacement for get string width is the glyph static plus a cast to int. Especially important if you use auto or use it to create rectangles

I think the answer is: font get width for some fonts and some strings gives a wrong answer and to retain compatability with old code that wrong answer should be retained. That was the argument presented for the font no arg deprecation here Font(FontOptions) constructor gives bigger fonts - #10 by baconpaul which has other chats about that choice

13

Whoa I just realized something which might take my tone from ‘friendly feedback’ to 'hey you folks kinda messed this one up a bit.`

And its this

juce::GlyphArrangement::getStringWidth doesn’t seem to be a function in JUCE 8.0.1.

This means there is basically no way to stage this change if you use JUCE for, say, two projects and share code between them.

I mean there is, In my shared GUI library I’m about to do a

#define STRINGW(a, b)
if JUCE < 801
a.stringwidth(b)
else
juce::Glyph::stringwidth(a,b)

its either that or lose the ability to provide shared code to people who use JUCE < 802 or turn off deprecation warnings. You see that this really is bad right?

But I really think that a single point release which deprecates a function and tells you to use a function which isn’t available in any prior version is really really bad news.

Perhaps a good semantic would be “if you are going to deprecate function A for function B” either (1) make sure function B already exists and has been in a release or (2) wait for a major not a point release.

14

We’ve had to do the same as we have about a dozen modules that are all on different juce versions as it takes a long time to update them.

The easiest way I found was to create a free function getStringWidth (Font, String) (and a float version) and have two versions depending on the juce version (forwarding to either Font or GlyphArrangement).

I have to add that I think this breaking change was made because of a bug report that I made where drawFittedText didn’t break lines anymore and drew outside the given rect. This made our UI have text overlapping everywhere and wasn’t shippable.

At least, it came in the same couple of days.

I think the rationale for this breaking change was that Font::getStringWidth couldn’t take in to account fallback fonts. So the result would be different from GlyphArrangement::getStringWidth which could.

However, if there’s no way to draw text without fallback fonts now (?), I don’t see why just forwarding from the old method wasn’t an acceptable solution?

15

Yeah I have this code going through CI now and added separate CI checks for 801 and 802.


#if JUCE_VERSION >= 0x080002
#define SST_STRING_WIDTH_INT(a, b) juce::GlyphArrangement::getStringWidthInt(a, b)
#define SST_STRING_WIDTH_FLOAT(a, b) juce::GlyphArrangement::getStringWidth(a, b)
#else
#define SST_STRING_WIDTH_INT(a, b) a.getStringWidth(b)
#define SST_STRING_WIDTH_FLOAT(a, b) a.getStringWidthFloat(b)
#endif
16

and look the bug fix is great! I would have loved it if there was an 8.0.2 which said “this bug occurs and to fix it shift from font.blah to glyph::blah. We will deprecate font.blah in 8.1”

but the all-in-one-shot-no-way-back thing is really really hard going if you want to reuse code. and so we all end up making version specific free functions ourselves

17

I should also add that I do understand the juce position. And tbh my default position is that code should be correct even if it breaks stuff*, otherwise it doesn’t progress and things remain broken (see also Hyrums Law).

*within reason

But juce has so many users each with different priorities. As we have several large interdependent code bases, we feel breaking changes a lot.
Others prioritise pixel perfect stability, others performance. I’m not sure there is a “right” choice here.

I do like the idea of macro-based deprecations that we can opt in to though. That would certainly be less intrusive and give us more time to transition.

Also, as an interesting data point, we have one product that we can’t ship on J8 as the Direct2D renderer isn’t fast enough for it yet and other that we have to ship with J8 as we need the speed of the Direct2D renderer.

And the first depends on the second for some functionality so we have to make the latter work with both J7 and J8.

Fun times!

18

yeah I think in this case three things would have helped with the transition

  1. macro based deprecations
  2. deprecation defaults on at a x.y not x.y.z release
  3. if function f is deprecated in favor of function g in x.y, then g exists in x.y-1
19

It’s obvious that the implementation of GlyphArrangement has to depend on Font. If we were to make the implementation of Font::getStringWidth depend on GlyphArrangement, we’d end up with a circular dependency between these two components, which would make future refactoring and maintenance more expensive. I’ve been bitten enough by this in the past that I try to avoid adding circular dependencies in new code wherever possible.

20

but anyway i got it all working and we are now headed to 8.0.2 for all our headline properties, which is good, with code which still works on 7 for our laggards.

21

Font specifics aside, I would love to make sure the JUCE team’s takeaway isn’t to deprecate less often — I’ve been really appreciative of the frequency lately and would welcome more “benign” deprecations that improves API clarity and consistency over the long term.

It does seem like a bit more structure would help — a common theme for many bitten here is “warnings are errors in CI” and “maintaining multiple JUCE versions” — the former is more and more common, and boils down to “deprecations break builds across the user base.”

For me personally, putting deprecations on “minor” instead of “patch” doesn’t really make too much difference. My read on the font issues is that JUCE team don’t develop complex apps/dependencies that span multiple JUCE versions, so it’s hard to assess impact of changes until they are already out. Ideally the team could get some input/iteration before they are set in stone — the lightweight version of this could be a policy to post on the forum tagging a few deprecation-sensitive parties for feedback on develop.