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 2 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
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 ?? false,
};

return new CoreTextNode(this, resolvedProps);
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 &&
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 false
*/
wrapWord: boolean;
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