I’ve just noticed that all my tooltips are misaligned. They’re set to centred, but only the last line is placed right. All the rest is shifted some pixels to the left. I think the newlines possibly added when making the layout are being counted. I can’t really follow all the stuff in TextLayout.cpp, but if I swap the parts of this conditional
so that it becomes
if (t.isWhitespace || t.isNewLine)
{
++charPosition;
}
else if (newGlyphs.size() > 0)
{
// etc
}
it gets fixed. Not sure if it’s the right fix though.
Ok it’s not any added newlines, it’s just the spaces after each last word. I still can’t get my head around TextLayout, but I can’t see how the check for whitespace would be reached at the else when all spaces are separate tokens with newGlyphs.size() == 1.
This is from the widgets demo, with some keysmashes added to make it multiline. It’s using native layout through DirectWrite, so TokenList::createLayout is not involved… There’s a difference though -the last line is shifted too.
Any clue? I’ll keep my edit for now, but it doesn’t seem like the right fix…
No one has seen this before? It’s worsened in my case by using a monospaced font, but still… I’m not even sure if it’s normal behaviour and I’m doing something wrong.
Bump. To restate the issue: TextLayout includes all spaces at the end of wrapped lines, so multiline centered tooltips are off-centre. I’m keeping a fork just for this one
Are you saying that the original change you posted fixes the issue for you? I think in your case TokenList::createLayout() is in fact being used because the call to TextLayout::createNativeLayout() in TextLayout::createLayout() is returning false for your custom font. Can you verify that this is the case?
I think that change makes sense and it seems to work OK in testing and fix the horizontal misalignment for JUCE-based text layouts but it’s hard to know whether there might be edge-cases where it could cause issues. The change means that we skip all whitespace and new-line glyphs, not just those at the end of lines, but I don’t think they are required to render the lines correctly.
Yes, the original change fixes it in my case, because indeed I’m using a custom font. What made me think of that change is that, as it is now, I never reach the else branch -it would need a whitespace / newline token with no glyphs. So one would think that the original intention was to check for whitespace / newline first, but I can’t be sure of that. The issue seems to happen also with DirectWrite, with the difference that all lines are wrong, including the last one. I tried to look there but got lost in the code.
Hmm I’m not seeing this. With your suggested change the JUCE and DirectWrite layouts are the same and seem to not have the horizontal offset issue. In the DirectWrite code we’re mostly relying on the native code to do the actual laying-out of the text after setting the DWRITE_TEXT_ALIGNMENT property so it should be accurate. Have you code a simple example which reproduces the DirectWrite issue?
struct MyAudioProcessorEditor : juce::AudioProcessorEditor
{
juce::TextLayout tl;
MyAudioProcessorEditor (MyAudioProcessor& p) : AudioProcessorEditor{ p }
{
juce::AttributedString s;
s.setJustification (juce::Justification::centred);
s.append ("You must know, then, that the above-named gentleman whenever he was at leisure (which was mostly all the year round) gave himself up to reading books of chivalry with such ardour and avidity that he almost entirely neglected the pursuit of his field-sports, and even the management of his property; and to such a pitch did his eagerness and infatuation go that he sold many an acre of tillageland to buy books of chivalry to read, and brought home as many of them as he could get.");
tl.createLayoutWithBalancedLineLengths (s, 400);
setSize (tl.getWidth(), tl.getHeight());
}
void paint (juce::Graphics& g) override
{
g.fillAll (juce::Colours::white);
tl.draw (g, getLocalBounds().toFloat());
}
JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (MyAudioProcessorEditor)
};
Interesting, thanks for looking into this. I’ve been doing some debugging and it seems that DirectWrite will call DrawGlyphRun() to render the trailing whitespace as a separate run, so adding this check to the top of that method also fixes the alignment for me:
const String runString (runDescription->string, runDescription->stringLength);
if (! runString.containsNonWhitespaceChars())
return S_OK;
One more thing about the standard layout case. I was inverting the conditional like
if (t.isWhitespace || t.isNewLine)
{
++charPosition;
}
else if (newGlyphs.size() > 0) // ...
but whitespace tokens can have a size > 1, in which case ++charPosition may be wrong. I’ve actually not been able to get a visible error for it, even for not incrementing charPosition at all. I don’t quite understand how charPosition works, given that
charPosition only seems to be used to set the TextLayout::Run::stringRange which as far as I can see isn’t used in positiong the text. Still, it’s safer to increment it correctly instead of always assuming a single glyph. In testing I was using this check which is working nicely:
if (newGlyphs.size() > 0)
{
if (! t.isWhitespace && ! t.isNewLine)
{
currentRun->glyphs.ensureStorageAllocated (currentRun->glyphs.size() + newGlyphs.size());
auto tokenOrigin = t.area.getPosition().translated (0, t.font.getAscent());
if (needToSetLineOrigin)
{
needToSetLineOrigin = false;
currentLine->lineOrigin = tokenOrigin;
}
auto glyphOffset = tokenOrigin - currentLine->lineOrigin;
for (int j = 0; j < newGlyphs.size(); ++j)
{
auto x = xOffsets.getUnchecked (j);
currentRun->glyphs.add (TextLayout::Glyph (newGlyphs.getUnchecked (j),
glyphOffset.translated (x, 0),
xOffsets.getUnchecked (j + 1) - x));
}
}
charPosition += newGlyphs.size();
}
I’ll push this and a fix for the DirectWrite code too shortly.
