From e38aa8bec444c6d08551f63d1b05bee4b75a7221 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 14:11:12 +0800 Subject: [PATCH 1/8] Improve compaction of empty/blank TextComponents --- .../adventure/text/ComponentCompaction.java | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 693789767..4e6b5f201 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -48,6 +48,11 @@ 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(self)) { + optimized = optimized.style(simplifyStyleForBlank(self.style())); + } + // leaf nodes do not need to be further optimized - there is no point return optimized; } @@ -74,7 +79,21 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa // optimize all children final List 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 @@ -120,6 +139,11 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa } } + // no children, style can be further simplified if self is blank + if (childrenToAppend.isEmpty() && isBlank(self)) { + optimized = optimized.style(simplifyStyleForBlank(self.style())); + } + return optimized.children(childrenToAppend); } @@ -166,6 +190,41 @@ 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(Component component) { + if (component instanceof TextComponent) { + final TextComponent textComponent = (TextComponent) component; + + for (char c : textComponent.content().toCharArray()) { + 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(); + + builder.color(null); + builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET); + + return builder.build(); + } + private static TextComponent joinText(final TextComponent one, final TextComponent two) { return TextComponentImpl.create(two.children(), one.style(), one.content() + two.content()); } From 04d74c36470c612c9234216250b7d314bec02660 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 14:33:03 +0800 Subject: [PATCH 2/8] Add OBFUSCATED as blank component redundancy --- .../main/java/net/kyori/adventure/text/ComponentCompaction.java | 1 + 1 file changed, 1 insertion(+) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 4e6b5f201..2d75485d8 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -221,6 +221,7 @@ private static boolean isBlank(Component component) { builder.color(null); builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET); + builder.decoration(TextDecoration.OBFUSCATED, TextDecoration.State.NOT_SET); return builder.build(); } From 67f45cd1d1437a285fb8f418d6069861c97b26f2 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 14:38:52 +0800 Subject: [PATCH 3/8] Add explanation for blank component style simplification --- .../net/kyori/adventure/text/ComponentCompaction.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 2d75485d8..1af31f0ad 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -218,10 +218,18 @@ private static boolean isBlank(Component component) { */ 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, as these styles only affect glyph rendering builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET); 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(); } From 724ff8710fa993ffa85adb054474ca955ef7ee26 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 14:40:13 +0800 Subject: [PATCH 4/8] Add clarification about obfuscated in modern versions --- .../main/java/net/kyori/adventure/text/ComponentCompaction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 1af31f0ad..b17d40521 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -222,7 +222,7 @@ private static boolean isBlank(Component component) { // TextColor doesn't affect spaces builder.color(null); - // ITALIC/OBFUSCATED don't affect spaces, as these styles only affect glyph rendering + // ITALIC/OBFUSCATED don't affect spaces (in modern versions), as these styles only affect glyph rendering builder.decoration(TextDecoration.ITALIC, TextDecoration.State.NOT_SET); builder.decoration(TextDecoration.OBFUSCATED, TextDecoration.State.NOT_SET); From 5a31a817fc6debbad0b43da9df1a9f6b5e9d1741 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 14:41:24 +0800 Subject: [PATCH 5/8] Avoid array allocation by using #charAt instead of #toCharArray --- .../java/net/kyori/adventure/text/ComponentCompaction.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index b17d40521..5c4bf3ec2 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -199,8 +199,10 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa private static boolean isBlank(Component component) { if (component instanceof TextComponent) { final TextComponent textComponent = (TextComponent) component; - - for (char c : textComponent.content().toCharArray()) { + + String content = textComponent.content(); + for (int i = 0; i < content.length(); i++) { + char c = content.charAt(i); if (c != ' ') return false; } From c99bfdb9a9c1fe1537377cd3a223db7518738c21 Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 15:03:01 +0800 Subject: [PATCH 6/8] Use instead of --- .../adventure/text/ComponentCompaction.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 5c4bf3ec2..51852e041 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -49,8 +49,8 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa if (childrenSize == 0) { // no children, style can be further simplified if self is blank - if (isBlank(self)) { - optimized = optimized.style(simplifyStyleForBlank(self.style())); + if (isBlank(optimized)) { + optimized = optimized.style(simplifyStyleForBlank(optimized.style())); } // leaf nodes do not need to be further optimized - there is no point @@ -58,8 +58,8 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa } // 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); @@ -140,8 +140,8 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa } // no children, style can be further simplified if self is blank - if (childrenToAppend.isEmpty() && isBlank(self)) { - optimized = optimized.style(simplifyStyleForBlank(self.style())); + if (childrenToAppend.isEmpty() && isBlank(optimized)) { + optimized = optimized.style(simplifyStyleForBlank(optimized.style())); } return optimized.children(childrenToAppend); @@ -191,18 +191,19 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa } /** - * Checks whether the Component is blank (a TextComponent containing only space characters) + * 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(Component component) { + private static boolean isBlank(final Component component) { if (component instanceof TextComponent) { final TextComponent textComponent = (TextComponent) component; - String content = textComponent.content(); + final String content = textComponent.content(); + for (int i = 0; i < content.length(); i++) { - char c = content.charAt(i); + final char c = content.charAt(i); if (c != ' ') return false; } @@ -213,7 +214,7 @@ private static boolean isBlank(Component component) { /** * Simplify the provided style to remove any information that is redundant, - * given that the content is blank + * given that the content is blank. * * @param style style to simplify * @return a new, simplified style From 1619c1f78d43fa13bdd5aa938bf6555cb5cdd52d Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 15:20:09 +0800 Subject: [PATCH 7/8] Add tests for blank compaction --- .../text/ComponentCompactingTest.java | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/api/src/test/java/net/kyori/adventure/text/ComponentCompactingTest.java b/api/src/test/java/net/kyori/adventure/text/ComponentCompactingTest.java index 2a1c51e89..6166bfe69 100644 --- a/api/src/test/java/net/kyori/adventure/text/ComponentCompactingTest.java +++ b/api/src/test/java/net/kyori/adventure/text/ComponentCompactingTest.java @@ -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; @@ -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()); + } + } From 7e6030758cae96489abd1013ddaa3693fdb261ac Mon Sep 17 00:00:00 2001 From: Moulberry Date: Sat, 14 May 2022 16:51:42 +0800 Subject: [PATCH 8/8] Slightly optimize compact by avoiding unnecessary work for non-text-components --- .../adventure/text/ComponentCompaction.java | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java index 51852e041..be3ac6ead 100644 --- a/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java +++ b/api/src/main/java/net/kyori/adventure/text/ComponentCompaction.java @@ -97,21 +97,23 @@ static Component compact(final @NotNull Component self, final @Nullable Style pa } // 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; + } } } @@ -121,22 +123,26 @@ 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