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.

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

Improve compaction of empty/blank TextComponents #764

Merged
merged 8 commits into from May 20, 2022
137 changes: 107 additions & 30 deletions api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java
Expand Up @@ -48,13 +48,18 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa
final int childrenSize = children.size();

if (childrenSize == 0) {
// no children, style can be further simplified if self is blank
if (isBlank(optimized)) {
optimized = optimized.style(simplifyStyleForBlank(optimized.style()));
}

// leaf nodes do not need to be further optimized - there is no point
return optimized;
}

// if there is only one child, check if self a useless empty component
if (childrenSize == 1 && self instanceof TextComponent) {
final TextComponent textComponent = (TextComponent) self;
if (childrenSize == 1 && optimized instanceof TextComponent) {
final TextComponent textComponent = (TextComponent) optimized;

if (textComponent.content().isEmpty()) {
final Component child = children.get(0);
Expand All @@ -74,25 +79,41 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa
// optimize all children
final List<Component> childrenToAppend = new ArrayList<>(children.size());
for (int i = 0; i < children.size(); ++i) {
childrenToAppend.add(compact(children.get(i), childParentStyle));
Component child = children.get(i);

// compact child recursively
child = compact(child, childParentStyle);

// ignore useless empty children (regardless of its style)
if (child.children().isEmpty() && child instanceof TextComponent) {
final TextComponent textComponent = (TextComponent) child;

if (textComponent.content().isEmpty()) {
continue;
}
}

childrenToAppend.add(child);
}

// try to merge children into this parent component
while (!childrenToAppend.isEmpty()) {
final Component child = childrenToAppend.get(0);
final Style childStyle = child.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);

if (optimized instanceof TextComponent && child instanceof TextComponent && Objects.equals(childStyle, childParentStyle)) {
// merge child components into the parent if they are a text component with the same effective style
// in context of their parent style
optimized = joinText((TextComponent) optimized, (TextComponent) child);
childrenToAppend.remove(0);

// if the merged child had any children, retain them
childrenToAppend.addAll(0, child.children());
} else {
// this child can't be merged into the parent, so all children from now on must remain children
break;
if (optimized instanceof TextComponent) {
while (!childrenToAppend.isEmpty()) {
final Component child = childrenToAppend.get(0);
final Style childStyle = child.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);

if (child instanceof TextComponent && Objects.equals(childStyle, childParentStyle)) {
// merge child components into the parent if they are a text component with the same effective style
// in context of their parent style
optimized = joinText((TextComponent) optimized, (TextComponent) child);
childrenToAppend.remove(0);

// if the merged child had any children, retain them
childrenToAppend.addAll(0, child.children());
} else {
// this child can't be merged into the parent, so all children from now on must remain children
break;
}
}
}

Expand All @@ -102,22 +123,31 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa
final Component child = childrenToAppend.get(i);
final Component neighbor = childrenToAppend.get(i + 1);

// calculate the children's styles in context of their parent style
final Style childStyle = child.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);
final Style neighborStyle = neighbor.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);
if (child.children().isEmpty() && child instanceof TextComponent && neighbor instanceof TextComponent) {
// calculate the children's styles in context of their parent style
final Style childStyle = child.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);
final Style neighborStyle = neighbor.style().merge(childParentStyle, Style.Merge.Strategy.IF_ABSENT_ON_TARGET);

if (child.children().isEmpty() && child instanceof TextComponent && neighbor instanceof TextComponent && childStyle.equals(neighborStyle)) {
final Component combined = joinText((TextComponent) child, (TextComponent) neighbor);
// check if styles are equivalent
if (childStyle.equals(neighborStyle)) {
final Component combined = joinText((TextComponent) child, (TextComponent) neighbor);

// replace the child and its neighbor with the single, combined component
childrenToAppend.set(i, combined);
childrenToAppend.remove(i + 1);
// replace the child and its neighbor with the single, combined component
childrenToAppend.set(i, combined);
childrenToAppend.remove(i + 1);

// don't increment the index -
// we want to try and optimize this combined component even further
} else {
i++;
// don't increment the index -
// we want to try and optimize this combined component even further
continue;
}
}

i++;
}

// no children, style can be further simplified if self is blank
if (childrenToAppend.isEmpty() && isBlank(optimized)) {
optimized = optimized.style(simplifyStyleForBlank(optimized.style()));
}

return optimized.children(childrenToAppend);
Expand Down Expand Up @@ -166,6 +196,53 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa
return builder.build();
}

/**
* Checks whether the Component is blank (a TextComponent containing only space characters).
*
* @param component the component to check
* @return true if the provided component is blank, false otherwise
*/
private static boolean isBlank(final Component component) {
if (component instanceof TextComponent) {
final TextComponent textComponent = (TextComponent) component;

final String content = textComponent.content();

for (int i = 0; i < content.length(); i++) {
final char c = content.charAt(i);
if (c != ' ') return false;
}

return true;
}
return false;
}

/**
* Simplify the provided style to remove any information that is redundant,
* given that the content is blank.
*
* @param style style to simplify
* @return a new, simplified style
*/
private static @NotNull Style simplifyStyleForBlank(final @NotNull Style style) {
final Style.Builder builder = style.toBuilder();

// TextColor doesn't affect spaces
builder.color(null);

// ITALIC/OBFUSCATED don't affect spaces (in modern versions), as these styles only affect glyph rendering
builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET);
zml2008 marked this conversation as resolved.
Show resolved Hide resolved
builder.decoration(TextDecoration.OBFUSCATED, TextDecoration.State.NOT_SET);

// UNDERLINE/STRIKETHROUGH affects spaces because the line renders on top
// BOLD affects spaces because it increments the character advance by 1

// font affects spaces in 1.19+ (since 22w11a), due to the font glyph provider for spaces

return builder.build();
}

private static TextComponent joinText(final TextComponent one, final TextComponent two) {
return TextComponentImpl.create(two.children(), one.style(), one.content() + two.content());
}
Expand Down
Expand Up @@ -24,8 +24,10 @@
package net.kyori.adventure.text;

import java.util.stream.Stream;
import net.kyori.adventure.key.Key;
import net.kyori.adventure.text.format.NamedTextColor;
import net.kyori.adventure.text.format.Style;
import net.kyori.adventure.text.format.TextColor;
import net.kyori.adventure.text.format.TextDecoration;
import org.junit.jupiter.api.DynamicTest;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -314,4 +316,84 @@ void testJoinTextWithChildren() {

assertEquals(expectedCompact, notCompact.compact());
}

@Test
void testBlankStyleRemoval() {
final String blank = " ";

final Component expectedCompact = text(blank);
final Component notCompact = text(blank, NamedTextColor.RED);

assertEquals(expectedCompact, notCompact.compact());
}

@Test
void testBlankCompactionWithRemovableStyle() {
final String blank = " ";

final Component expectedCompact = text(blank);
final Component notCompact = text().append(
text(blank, NamedTextColor.RED)
).build();

assertEquals(expectedCompact, notCompact.compact());
}

@Test
void testBlankCompactionWithManyStyle() {
final String blank = " ";

final Component expectedCompact = text(blank + blank + blank + blank);
final Component notCompact = text().append(
text(blank, NamedTextColor.RED)
.append(text(blank).decorate(TextDecoration.ITALIC))
).append(
text(blank).decorate(TextDecoration.OBFUSCATED)
.append(text(blank, TextColor.color(0xDEADB33F)))
).build();

assertEquals(expectedCompact, notCompact.compact());
}

@Test
void testBlankCompactionWithNonRemovableStyle() {
final String blank = " ";

final Component expectedCompact = text(blank).decorate(TextDecoration.STRIKETHROUGH);
final Component notCompact = text().append(
text(blank).decorate(TextDecoration.STRIKETHROUGH)
).build();

assertEquals(expectedCompact, notCompact.compact());
}

@Test
void testBlankCompactionWithManyNonRemovableStyle() {
final String blank = " ";

final Component notCompact = text().append(
text(blank).decorate(TextDecoration.STRIKETHROUGH)
.append(text(blank).decorate(TextDecoration.BOLD))
).append(
text(blank).decorate(TextDecoration.UNDERLINED)
.append(text(blank).font(Key.key("kyori:meow")))
).build();

assertEquals(notCompact, notCompact.compact());
}

@Test
void testEmptyCompactionWithManyNonRemovableStyle() {
final Component expectedCompact = text("meow");
final Component notCompact = text().content("meow").append(
empty().decorate(TextDecoration.STRIKETHROUGH)
.append(empty().decorate(TextDecoration.BOLD))
).append(
empty().decorate(TextDecoration.UNDERLINED)
.append(empty().font(Key.key("kyori:meow")))
).build();

assertEquals(expectedCompact, notCompact.compact());
}

}