Allow framed texts' frames to not demand width greater than (or equal to) height

[Jojo-Schmitz]

I don't see any special reason why a (rectangular) frame around text can't be higher than wide, esp. for multi-line texts.
It there is one, please enlighten me...

This PR also renames FrameType::SQUARE to FrameType::RECANTGLE and TextBase::square() to TextBase::rectangle(), because that's what these really are.

Resolves: #30394

[Jojo-Schmitz]

The vtests failures are happening due to this PR, but IMHO are harmless, it makes some frames around rehearsal marks a bit narrower, a very tiny bit for "A", a bit more for "C"

[cbjeukendrup]

Non-square rehearsal mark borders don't look great to me... @its-not-nice what do you think?

[Jojo-Schmitz]

Check how they look with multi-line text...

[Jojo-Schmitz]

Maybe an exception for (single letter) rehearsal marks?

[cbjeukendrup]

Or maybe separate rectangle and square options?

[Jojo-Schmitz]

To clarify: currently it is a rectangle, just with the restriction that it cannot be narrower than high, i.e. it can be wider than high.

[Jojo-Schmitz]

And I'm more concerned about something (IMHO ugly) like this:
vs.

than about 1-line stuff

[Jojo-Schmitz]

Is this better? Only affects multi-line texts (with frames) now. Passes all vtests too...

[cbjeukendrup]

That might be a good compromise. However, let's wait for @its-not-nice's opinion.

[Jojo-Schmitz]

It is very unlikely for @its-not-nice to not have an opinion (on this) ;-)

[its-not-nice]

I assume the height (for a single line) is calculated from the font metrics (cap height or body height or whatever) plus the margin. So maybe all we need to do is enforce that same value as a minimum width, but otherwise to follow the actual width of the text when it goes beyond that. Maybe that's what the PR does already, I haven't tested it myself yet.

There's also a relevant issue for this now! #30394

[Jojo-Schmitz]

So maybe all we need to do is enforce that same value as a minimum width, but otherwise to follow the actual width of the text when it goes beyond that

Minimum width of what? The frame goes around text, no text -> no frame. Same for height. If there is text (even if it is a space), its width (or height) is taken, plus the margins.

rebased (and changed some new SQUARE to RECTANGLE)

[Jojo-Schmitz]

Actually there remains a(n empty) square if the text contains nothing but line feeds.

But that happened before this PR too

[Jojo-Schmitz]

Just came up on Discord: https://discord.com/channels/818804595450445834/818804809784229898/1428431078170820679

[Jojo-Schmitz]

Anything wrong with this approach?

[MarcSabatella]

To me it should a rectangle that fits the text, no special minimum or maximum, except for single-character rehearsal marks, which should probably be square. On the other hand, I don't love the idea of literally special-casing single-character rehearsal marks. Correct me if I'm wrong, but I think @its-not-nice was suggesting using the line height of the font as the minimum height and width for the frame, but other than that letting the size be determined by the text. That works for me. It means "I" and "a" would get the same size square frame whether used as rehearsal marks or not, and that's fine by me.

[cbjeukendrup]

using the line height of the font as the minimum height and width for the frame

Yes, that was the idea.

[Jojo-Schmitz]

Isn't that what this PR is doing?

[cbjeukendrup]

Not when there are multiple lines:

Both frames here should have the same width.

[MarcSabatella]

The PR special cases the number of lines; that's not what I think we are agreeing on here. It also uses the calculated element height rather than the line height from the font metrics, which in theory might mean "a" and "A" could have different sizes. It should just use the font metric line height as the minimum for both the height and width.

[Jojo-Schmitz]

OK, how and where?

[Jojo-Schmitz]

It uses the text's bounding box, so even on a single character and independant of this PR this results in different size frames, here 3 different single char rehearsal marks:

So which width to take on a multiline text?

and where to get it from. And where (and how) to set it?

[Jojo-Schmitz]

Further up that method uses this for empty text:

  if (ldata->bbox().width() <= 1.0 || ldata->bbox().height() < 1.0) {      // or bbox.width() <= 1.0
        // this does not work for Harmony:
        FontMetrics fm(font());
        double ch = fm.ascent();
        double cw = fm.width('n');
        ldata->frame = RectF(0.0, -ch, cw, ch);
    }

So uses the Font's ascent() and n's width for the height and width of the frame (which seems to result in a square for some reason that's beyond me, seems coincidentially an 'n' is half as wide as the font's ascent?)
Something to use for multiline text? Like never make the frame for multiline text narrower than an 'n'?

[MarcSabatella]

I would prefer to see no special casing based on number of lines at all. Just as simple as what was said above: take the line height of the font as the minimum for frame height and frame width, independently of the content of the element. So, calculation the height and width normally, then for both of those take a "max" of the calculated value and the line height of the font. This will allows "a" and "A" to have the same square box, and "I" the same width as "A", and normal text will just work regardless of number of lines.

A vertical column of "I" characters should probably just as wide as a single "I", although I guess if someone had a real world use case in mind where it made sense to have a tighted width for the column, then it could make sense only apply that min for single-line texts. But right now that seems an unnecessary complication for no gain.

As for how to get the font line height, I'm not versed enough in how things are done currently to say. That "fm.ascent" certainly seems promising, but I imagine there could be a complication if the text in question contains multiple fonts. But, whatever. we're just using this as a min value, if the text itself needs to be bigger, it will be.

[Jojo-Schmitz]

Split into 2 commits, one for the renaming of square to rectrangle and another for the (very simple) fix to allow for non-square frames for multi-line. Otherwise unchanged. It basically is a special case for having square frames on single letter framed texts, so especially for rehearsal marks. All other frames just follow their content, in width and height.
I've wasted quite many hours yesterday in an attempt to get some minimum width to work, without getting anywhere. Not sure I'm willing to waste more time on it...
Maybe that'd be something for a different PR (by someone else)?

[Jojo-Schmitz]

Well, here's another attempt... (will merge the 2nd and 3rd commit if it passes muster)
Should now look like this:

(Images created with my 3.7, as I can't run master without #31052 being merged), exactly the same width with lower case 'i', very slighly different with uppercase 'I'. Downside: a single (lowercase) 'j' and a 'w' now don't have a square frame anymore...

Strange enough these 2 seem to be the only exceptions (within a-z,A-Z,0-9 and using the default font, Edwin)
The 'w' situation isn't new though.

[Jojo-Schmitz]

When using height() (ascent() + descend()) rather than (just) ascent(), the situation for "j" improves (to a square), but also the min. width increases a bit for some multi-line text borders (not too bad though).
The "w" issue still remains, but, as said above, is not new, might need a special casing and a separate issue.