Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement wrapWord parameter (#268) #345

Draft
wants to merge 8 commits into
base: dev
Choose a base branch
from
Draft

Implement wrapWord parameter (#268) #345

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
da89c2e
Add wrapWord option, no implementation yet
marcel-danilewicz-consult-red Jul 12, 2024
98b45c9
Implement wrapWord for sdf text renderer
marcel-danilewicz-consult-red Jul 17, 2024
8a939ad
Change wrapWord to string literal
marcel-danilewicz-consult-red Jul 26, 2024
4f47680
Implement wrapWord for canvas text renderer
marcel-danilewicz-consult-red Jul 29, 2024
c7f8c9c
Add wrapWord parameter to text-contain example
marcel-danilewicz-consult-red Jul 29, 2024
856cc7c
Improve breaking algorithm
marcel-danilewicz-consult-red Jul 30, 2024
3c98b21
Change order of conditions
marcel-danilewicz-consult-red Jul 31, 2024
dab52d2
Align line breaking algorithm for wrapWord
marcel-danilewicz-consult-red Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions examples/tests/text-contain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ export default async function test(settings: ExampleSettings) {
fontFamily: 'Ubuntu',
textRendererOverride: 'sdf',
fontSize: 20,
text: `Lorem ipsum dolor sit e
text: `LoremipsumdolorsiteConsecteturadipiscingelit.Vivamusid.
Lorem ipsum dolor sit e
Consectetur adipiscing elit. Vivamus id.
Suspendisse sollicitudin posuere felis.
Vivamus consectetur ex magna, non mollis.`,
Expand Down Expand Up @@ -132,6 +133,7 @@ Vivamus consectetur ex magna, non mollis.`,
// SDF, contain none
text1.textRendererOverride = 'sdf';
text1.contain = 'none';
text1.wrapWord = 'normal';
text1.width = 0;
text1.height = 0;
},
Expand All @@ -153,11 +155,17 @@ Vivamus consectetur ex magna, non mollis.`,
// SDF, contain both (1 pixel larger to show another line)
text1.height = 204;
},
() => {
// SDF, contain both (1 pixel larger to show another line), wrap word
text1.contain = 'width';
text1.wrapWord = 'break';
},

() => {
// Canvas, contain none
text1.textRendererOverride = 'canvas';
text1.contain = 'none';
text1.width = 0;
(text1.wrapWord = 'normal'), (text1.width = 0);
text1.height = 0;
},
() => {
Expand All @@ -180,6 +188,11 @@ Vivamus consectetur ex magna, non mollis.`,
// Canvas, contain both (1 pixel larger to show another line)
text1.height = 204;
},
() => {
// Canvas, contain both (1 pixel larger to show another line), wrap word
text1.contain = 'width';
text1.wrapWord = 'break';
},
];
/**
* Run the next mutation in the list
Expand All @@ -202,6 +215,7 @@ Vivamus consectetur ex magna, non mollis.`,
text1.contain,
text1.width,
text1.height,
text1.wrapWord,
);
indexInfo.text = (i + 1).toString();
textSetDimsInfo.text = `Set size: ${Math.round(text1.width)}x${Math.round(
Expand Down Expand Up @@ -237,6 +251,7 @@ function makeHeader(
contain: string,
width: number,
height: number,
wrapWord: string,
) {
return `${renderer}, contain = ${contain}`;
return `${renderer}, contain = ${contain}, wrapWord = ${wrapWord}`;
}
11 changes: 11 additions & 0 deletions src/core/CoreTextNode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export class CoreTextNode extends CoreNode implements CoreTextNodeProps {
textBaseline: props.textBaseline,
verticalAlign: props.verticalAlign,
overflowSuffix: props.overflowSuffix,
wrapWord: props.wrapWord,
});
this.textRenderer = resolvedTextRenderer;
this.trState = textRendererState;
Expand Down Expand Up @@ -346,6 +347,16 @@ export class CoreTextNode extends CoreNode implements CoreTextNodeProps {
}
}

get wrapWord(): CoreTextNodeProps['wrapWord'] {
return this.trState.props.wrapWord;
}

set wrapWord(value: CoreTextNodeProps['wrapWord']) {
if (this.textRenderer.set.wrapWord) {
this.textRenderer.set.wrapWord(this.trState, value);
}
}

get debug(): CoreTextNodeProps['debug'] {
return this.trState.props.debug;
}
Expand Down
1 change: 1 addition & 0 deletions src/core/Stage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ export class Stage {
overflowSuffix: props.overflowSuffix ?? '...',
debug: props.debug ?? {},
shaderProps: null,
wrapWord: props.wrapWord ?? 'normal',
};

return new CoreTextNode(this, resolvedProps);
Expand Down
1 change: 1 addition & 0 deletions src/core/text-rendering/renderers/CanvasTextRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ export class CanvasTextRenderer extends TextRenderer<CanvasTextRendererState> {
].join(' '),
textColor: getNormalizedRgbaComponents(state.props.color),
offsetY: state.props.offsetY,
wordBreak: state.props.wrapWord == 'break',
wordWrap: state.props.contain !== 'none',
wordWrapWidth:
state.props.contain === 'none' ? undefined : state.props.width,
Expand Down
15 changes: 15 additions & 0 deletions src/core/text-rendering/renderers/LightningTextTextureRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ export class LightningTextTextureRenderer {
wordWrapWidth,
letterSpacing,
textIndent,
this._settings.wordBreak,
);
} else {
linesInfo = { l: this._settings.text.split(/(?:\r\n|\r|\n)/), n: [] };
Expand All @@ -347,6 +348,7 @@ export class LightningTextTextureRenderer {
wordWrapWidth - w,
letterSpacing,
textIndent,
this._settings.wordBreak,
);
usedLines[usedLines.length - 1] = `${al.l[0]!}${
this._settings.overflowSuffix
Expand Down Expand Up @@ -681,6 +683,7 @@ export class LightningTextTextureRenderer {
wordWrapWidth: number,
letterSpacing: number,
indent = 0,
wordBreak: boolean,
) {
// Greedy wrapping algorithm that will wrap words as the line grows longer.
// than its horizontal bounds.
Expand Down Expand Up @@ -720,6 +723,18 @@ export class LightningTextTextureRenderer {
realNewlines.push(allLines.length);
}
}
if (wordBreak) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is incorrect in some cases, and inefficient in general. You should instead break words as part of the wrapText function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is inside of wrapText function :)
Could you give me an example of NG case for this code?

Copy link
Contributor

@elsassph elsassph Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but it is misplaced I think: it re-measures every word which we already measure in the above loop - could the break happen as part of the above wrapping logic? Also isn't that a different breaking logic as SDF? It seems SDF tries to cut the word to fit the line, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It remeasures whole lines. I'd rather separate normal and break cases, but you are right that the code can be optimized. Algorithms are not the same due to different surrounding code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No allLines has all the individual words, and realNewlines tells the index of each new line's first word.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the algorithm in last commit. It should be better now, but words still need to be measured in order to find the breaking point. Probably it can be optimized further , but I don't see a way to entirely avoid measurement.

for (let i = 0; i < allLines.length; i++) {
let newline = '';
while (this.measureText(allLines[i]!, letterSpacing) > wordWrapWidth) {
newline = allLines[i]!.slice(-1) + newline;
allLines[i] = allLines[i]!.slice(0, -1);
}
if (newline.length > 0) {
allLines.splice(i + 1, 0, newline);
}
}
}

return { l: allLines, n: realNewlines };
}
Expand Down
2 changes: 2 additions & 0 deletions src/core/text-rendering/renderers/SdfTextRenderer/SdfTextRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ export class SdfTextRenderer extends TextRenderer<SdfTextRendererState> {
scrollable,
overflowSuffix,
maxLines,
wrapWord,
} = state.props;

// scrollY only has an effect when contain === 'both' and scrollable === true
Expand Down Expand Up @@ -569,6 +570,7 @@ export class SdfTextRenderer extends TextRenderer<SdfTextRendererState> {
scrollable,
overflowSuffix,
maxLines,
wrapWord,
);

state.bufferUploaded = false;
Expand Down
19 changes: 19 additions & 0 deletions src/core/text-rendering/renderers/SdfTextRenderer/internal/layoutText.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export function layoutText(
scrollable: TrProps['scrollable'],
overflowSuffix: TrProps['overflowSuffix'],
maxLines: TrProps['maxLines'],
wrapWord: TrProps['wrapWord'],
): {
bufferNumFloats: number;
bufferNumQuads: number;
Expand Down Expand Up @@ -292,6 +293,24 @@ export function layoutText(
maxX = Math.max(maxX, quadX + glyph.width);
curX += glyph.xAdvance;
}
if (
marcel-danilewicz-consult-red marked this conversation as resolved.
Show resolved Hide resolved
wrapWord == 'break' &&
charEndX + glyph.width >= lineVertexW &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that correct? Wrap-word I think is meant to only wrap a word when this word alone is larger than the wrapping width.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it should be wrapped if the width is exceeded. So if a word doesn't fit it should be moved to the next line and then if still doesn't fit it should be wrapped?

Copy link
Contributor

@elsassph elsassph Jul 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be silly to break every word. The point is that sometimes (as reported in #268) you may have a single word larger than the target width, in which case you may prefer to break it.

In L2, that's what wordBreak option does. I recommend checking the implementation (which is TBH rather inefficient). The algorithm actually breaks words "before" doing the layout, so for instance a very long word would be always cut to not exceed the full width, but it won't try to fit a few characters on the previous line. It also supports words being multiple times longer than the available width - imagine for instance using this feature to render text vertically, one letter per line.
See: https://github.com/rdkcentral/Lightning/blob/dev/src/textures/TextTextureRendererAdvanced.mjs#L175

I suggest again to make sure that the requirements are non-ambiguous - ask @wouterlucas for confirmation. Make a diagram/drawing to explain what you are going to do.

In any case it is very wise to have proposed an early draft.

contain != 'none'
) {
if (curLineBufferStart !== -1 && lineIsWithinWindow) {
bufferLineInfos.push({
bufferStart: curLineBufferStart,
bufferEnd: bufferOffset,
});
curLineBufferStart = -1;
}
curX = 0;
curY += vertexLineHeight;
curLineIndex++;
lastWord.codepointIndex = -1;
xStartLastWordBoundary = 0;
}
} else {
// Unmapped character

Expand Down
9 changes: 9 additions & 0 deletions src/core/text-rendering/renderers/TextRenderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,12 @@ export interface TrProps extends TrFontProps {
* @default 'none'
*/
contain: 'none' | 'width' | 'both';
/**
* Word wrap option
*
* @default normal
*/
wrapWord: 'normal' | 'break';
width: number;
height: number;
/**
Expand Down Expand Up @@ -370,6 +376,9 @@ const trPropSetterDefaults: TrPropSetters = {
contain: (state, value) => {
state.props.contain = value;
},
wrapWord: (state, value) => {
state.props.wrapWord = value;
},
offsetY: (state, value) => {
state.props.offsetY = value;
},
Expand Down
Loading