From aed6837cc1e1bfb9ccb238900ad7387444b624eb Mon Sep 17 00:00:00 2001 From: Jaakko Keränen Date: Thu, 15 Jul 2021 18:18:45 +0300 Subject: Fixing regressions text metrics, InputWidget `run_Font_` was moving the Y cursor position twice for each line break. Checking for the HarfBuzz UNSAFE_TO_BREAK flag leads to some unexpected behavior near edges of words. The old `tryAdvanceNoWrap` method should return the maximum horizontal advance of the text, and not the cursor position's advance. `draw_WrapText` used the wrong foreground color. `TextBuf` now uses WrapText to do all the measuring and drawing, making things much simpler. --- res/about/version.gmi | 2 +- src/ui/inputwidget.c | 68 +++++++++++++++------------------------------------ src/ui/text.c | 23 ++++++++++++----- src/ui/text.h | 4 +++ 4 files changed, 42 insertions(+), 55 deletions(-) diff --git a/res/about/version.gmi b/res/about/version.gmi index 89c9bedc..a39d792b 100644 --- a/res/about/version.gmi +++ b/res/about/version.gmi @@ -18,7 +18,7 @@ * Preferences have been reorganized. There is a new Fonts page, and some General settings have been moved to the Style page. All color-related settings are on the Colors page, including UI theme colors. * Opened links are updated to reflect visited status even when opened in a background tab or to the side. * Unicode Byte Order Mark is ignored in the beginning of a page. -* Right-to-left paragraphs are aligned to the right. +* Right-to-left paragraphs are aligned to the right. Link icons and list/quote symbols are moved to the right margin. * Large lede paragraph font is not applied if the paragraph has too many lines. * Adjusted kerning of Nunito. * Fixed sizing of characters in the Noto Symbols fonts. diff --git a/src/ui/inputwidget.c b/src/ui/inputwidget.c index b51b3985..29003ad5 100644 --- a/src/ui/inputwidget.c +++ b/src/ui/inputwidget.c @@ -747,7 +747,13 @@ static size_t indexForRelativeX_InputWidget_(const iInputWidget *d, int x, const static iBool moveCursorByLine_InputWidget_(iInputWidget *d, int dir) { const iInputLine *line = line_InputWidget_(d, d->cursorLine); - int xPos = measureN_Text(d->font, cstr_String(&line->text), d->cursor - line->offset).advance.x; + int xPos1 = maxWidth_TextMetrics(measureN_Text(d->font, + cstr_String(&line->text), + d->cursor - line->offset)); + int xPos2 = maxWidth_TextMetrics(measureN_Text(d->font, + cstr_String(&line->text), + iMin(d->cursor + 1 - line->offset, line->len))); + const int xPos = (xPos1 + xPos2) / 2; size_t newCursor = iInvalidPos; const size_t numLines = size_Array(&d->lines); if (dir < 0 && d->cursorLine > 0) { @@ -861,32 +867,6 @@ static size_t skipWord_InputWidget_(const iInputWidget *d, size_t pos, int dir) return pos; } -#if 0 -static iInt2 textOrigin_InputWidget_(const iInputWidget *d) { //}, const char *visText) { -// const iWidget *w = constAs_Widget(d); - iRect bounds = contentBounds_InputWidget_(d);/* adjusted_Rect(bounds_Widget(w), - addX_I2(padding_(), d->leftPadding), - neg_I2(addX_I2(padding_(), d->rightPadding)));*/ -// const iInt2 emSize = advance_Text(d->font, "M"); -// const int textWidth = advance_Text(d->font, visText).x; -// const int cursorX = advanceN_Text(d->font, visText, d->cursor).x; -// int xOff = 0; -// shrink_Rect(&bounds, init_I2(gap_UI * (flags_Widget(w) & tight_WidgetFlag ? 1 : 2), 0)); -/* if (d->maxLen == 0) { - if (textWidth > width_Rect(bounds) - emSize.x) { - xOff = width_Rect(bounds) - emSize.x - textWidth; - } - if (cursorX + xOff < width_Rect(bounds) / 2) { - xOff = width_Rect(bounds) / 2 - cursorX; - } - xOff = iMin(xOff, 0); - }*/ -// const int yOff = 0.3f * lineHeight_Text(d->font); // (height_Rect(bounds) - lineHeight_Text(d->font)) / 2; -// return addY_I2(topLeft_Rect(bounds), yOff); - -} -#endif - static size_t coordIndex_InputWidget_(const iInputWidget *d, iInt2 coord) { const iInt2 pos = sub_I2(coord, contentBounds_InputWidget_(d).pos); const size_t lineNumber = iMin(iMax(0, pos.y) / lineHeight_Text(d->font), @@ -1342,17 +1322,6 @@ static iBool processEvent_InputWidget_(iInputWidget *d, const SDL_Event *ev) { return processEvent_Widget(w, ev); } -#if 0 -static iBool isWhite_(const iString *str) { - iConstForEach(String, i, str) { - if (!isSpace_Char(i.value)) { - return iFalse; - } - } - return iTrue; -} -#endif - static void draw_InputWidget_(const iInputWidget *d) { const iWidget *w = constAs_Widget(d); iRect bounds = adjusted_Rect(bounds_InputWidget_(d), padding_(), neg_I2(padding_())); @@ -1400,14 +1369,12 @@ static void draw_InputWidget_(const iInputWidget *d) { /* Draw the selected range. */ const iRanges mark = mark_InputWidget_(d); if (mark.start < lineRange.end && mark.end > lineRange.start) { - const int m1 = measureN_Text(d->font, + const int m1 = maxWidth_TextMetrics(measureN_Text(d->font, cstr_String(&line->text), - iMax(lineRange.start, mark.start) - line->offset) - .advance.x; - const int m2 = measureN_Text(d->font, + iMax(lineRange.start, mark.start) - line->offset)); + const int m2 = maxWidth_TextMetrics(measureN_Text(d->font, cstr_String(&line->text), - iMin(lineRange.end, mark.end) - line->offset) - .advance.x; + iMin(lineRange.end, mark.end) - line->offset)); fillRect_Paint(&p, (iRect){ addX_I2(drawPos, iMin(m1, m2)), init_I2(iMax(gap_UI / 3, iAbs(m2 - m1)), @@ -1446,15 +1413,20 @@ static void draw_InputWidget_(const iInputWidget *d) { } const iInputLine *curLine = line_InputWidget_(d, d->cursorLine); const iString * text = &curLine->text; - /* The `gap_UI` offsets below are a hack. They are used because for some reason the - cursor rect and the glyph inside don't quite position like during `run_Text_()`. */ - const int prefixSize = measureN_Text(d->font, cstr_String(text), d->cursor - curLine->offset).advance.x; + /* The bounds include visible characters, while advance includes whitespace as well. + Normally only the advance is needed, but if the cursor is at a newline, the advance + will have reset back to zero. */ + const int prefixSize = maxWidth_TextMetrics(measureN_Text(d->font, + cstr_String(text), + d->cursor - curLine->offset)); const iInt2 curPos = addX_I2(addY_I2(contentBounds.pos, lineHeight_Text(d->font) * d->cursorLine), prefixSize + (d->mode == insert_InputMode ? -curSize.x / 2 : 0)); - const iRect curRect = { curPos, curSize }; + const iRect curRect = { curPos, curSize }; fillRect_Paint(&p, curRect, uiInputCursor_ColorId); if (d->mode == overwrite_InputMode) { + /* The `gap_UI` offsets below are a hack. They are used because for some reason the + cursor rect and the glyph inside don't quite position like during `run_Text_()`. */ draw_Text(d->font, addX_I2(curPos, iMin(1, gap_UI / 8)), uiInputCursorText_ColorId, diff --git a/src/ui/text.c b/src/ui/text.c index 0a3b7b2e..b283d700 100644 --- a/src/ui/text.c +++ b/src/ui/text.c @@ -1450,7 +1450,7 @@ static iRect run_Font_(iFont *d, const iRunArgs *args) { wrapResumePos = run->logical.end; wrapRuns.end = runIndex; wrapResumeRunIndex = runIndex + 1; - yCursor += d->height; + //yCursor += d->height; break; } wrapResumeRunIndex = runCount; @@ -1489,10 +1489,10 @@ static iRect run_Font_(iFont *d, const iRunArgs *args) { prevCh = ch; } else { - if (~glyphFlags & HB_GLYPH_FLAG_UNSAFE_TO_BREAK) { + //if (~glyphFlags & HB_GLYPH_FLAG_UNSAFE_TO_BREAK) { safeBreakPos = logPos; breakAdvance = wrapAdvance; - } + //} } /* Out of room? */ if (wrapAdvance + xOffset + glyph->d[0].x + glyph->rect[0].size.x > @@ -1787,7 +1787,8 @@ iInt2 tryAdvanceNoWrap_Text(int fontId, iRangecc text, int width, const char **e .maxWidth = width, .wrapFunc = cbAdvanceOneLine_, .context = endPos }; - return measure_WrapText(&wrap, fontId).bounds.size; + iTextMetrics tm = measure_WrapText(&wrap, fontId); + return init_I2(maxWidth_TextMetrics(tm), tm.bounds.size.y); } iTextMetrics measureN_Text(int fontId, const char *text, size_t n) { @@ -1969,7 +1970,7 @@ void draw_WrapText(iWrapText *d, int fontId, iInt2 pos, int color) { .text = d->text, .pos = pos, .wrap = d, - .color = color, + .color = color & mask_ColorId, }); } @@ -2088,6 +2089,13 @@ iDefineTypeConstructionArgs(TextBuf, (int font, int color, const char *text), fo static void initWrap_TextBuf_(iTextBuf *d, int font, int color, int maxWidth, iBool doWrap, const char *text) { SDL_Renderer *render = text_.render; + iWrapText wrapText = { + .text = range_CStr(text), + .maxWidth = maxWidth, + .mode = (doWrap ? word_WrapTextMode : anyCharacter_WrapTextMode), + }; + d->size = measure_WrapText(&wrapText, font).bounds.size; +#if 0 if (maxWidth == 0) { d->size = measure_Text(font, text).bounds.size; } @@ -2101,6 +2109,7 @@ static void initWrap_TextBuf_(iTextBuf *d, int font, int color, int maxWidth, iB d->size.y += iMax(size.y, lineHeight_Text(font)); } } +#endif SDL_SetHint(SDL_HINT_RENDER_SCALE_QUALITY, "0"); if (d->size.x * d->size.y) { d->texture = SDL_CreateTexture(render, @@ -2119,7 +2128,7 @@ static void initWrap_TextBuf_(iTextBuf *d, int font, int color, int maxWidth, iB SDL_SetRenderDrawColor(render, 255, 255, 255, 0); SDL_RenderClear(render); SDL_SetTextureBlendMode(text_.cache, SDL_BLENDMODE_NONE); /* blended when TextBuf is drawn */ - const int fg = color | fillBackground_ColorId; +#if 0 iRangecc range = range_CStr(text); if (maxWidth == 0) { draw_Text_(font, zero_I2(), fg, range); @@ -2137,6 +2146,8 @@ static void initWrap_TextBuf_(iTextBuf *d, int font, int color, int maxWidth, iB pos.y += lineHeight_Text(font); } } +#endif + draw_WrapText(&wrapText, font, zero_I2(), color | fillBackground_ColorId); SDL_SetTextureBlendMode(text_.cache, SDL_BLENDMODE_BLEND); SDL_SetRenderTarget(render, oldTarget); SDL_SetTextureBlendMode(d->texture, SDL_BLENDMODE_BLEND); diff --git a/src/ui/text.h b/src/ui/text.h index 09d92ce0..d779dd96 100644 --- a/src/ui/text.h +++ b/src/ui/text.h @@ -159,6 +159,10 @@ struct Impl_TextMetrics { iInt2 advance; /* cursor offset */ }; +iLocalDef int maxWidth_TextMetrics(const iTextMetrics d) { + return iMax(width_Rect(d.bounds), d.advance.x); +} + iTextMetrics measureRange_Text (int fontId, iRangecc text); iTextMetrics measureWrapRange_Text (int fontId, int maxWidth, iRangecc text); iTextMetrics measureN_Text (int fontId, const char *text, size_t n); /* `n` in characters */ -- cgit v1.2.3