From 50b585e20ce33aa795983ecc663cc444164479ce Mon Sep 17 00:00:00 2001 From: Siyamed Sinir Date: Mon, 18 Dec 2017 18:47:37 -0800 Subject: [PATCH] Fix DynamicLayout block index calculation after edit DynamicLayout made wrong adjustments to the block indices set to be redrawn. Every indice after the first change was being added the diff count in the blocks. However this meant when there are 2 deletes and an index as 1, it would become -1 (wrong array index). The change should be applied to indices outside of [firstBlock, lastBlock]. Test: Added android.text.DynamicLayoutTest#testReflow_afterSpannableEdit Test: bit FrameworksCoreTests:android.text.DynamicLayoutTest Test: bit FrameworksCoreTests:android.text.DynamicLayoutBlocksTest Test: bit CtsTextTestCases:android.text.cts.DynamicLayoutTest Bug: 67405337 Change-Id: Ie5e5658ffca4293f32f7b574d22d307e9e8d52dc --- core/java/android/text/DynamicLayout.java | 26 ++++++++++++------- .../src/android/text/DynamicLayoutTest.java | 22 ++++++++++++++++ 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/core/java/android/text/DynamicLayout.java b/core/java/android/text/DynamicLayout.java index fba358cf4c1b3..71e039a1e0ef5 100644 --- a/core/java/android/text/DynamicLayout.java +++ b/core/java/android/text/DynamicLayout.java @@ -492,7 +492,9 @@ public class DynamicLayout extends Layout } } - private void reflow(CharSequence s, int where, int before, int after) { + /** @hide */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public void reflow(CharSequence s, int where, int before, int after) { if (s != mBase) return; @@ -805,8 +807,8 @@ public class DynamicLayout extends Layout return; } - int firstBlock = -1; - int lastBlock = -1; + /*final*/ int firstBlock = -1; + /*final*/ int lastBlock = -1; for (int i = 0; i < mNumberOfBlocks; i++) { if (mBlockEndLines[i] >= startLine) { firstBlock = i; @@ -821,10 +823,10 @@ public class DynamicLayout extends Layout } final int lastBlockEndLine = mBlockEndLines[lastBlock]; - boolean createBlockBefore = startLine > (firstBlock == 0 ? 0 : + final boolean createBlockBefore = startLine > (firstBlock == 0 ? 0 : mBlockEndLines[firstBlock - 1] + 1); - boolean createBlock = newLineCount > 0; - boolean createBlockAfter = endLine < mBlockEndLines[lastBlock]; + final boolean createBlock = newLineCount > 0; + final boolean createBlockAfter = endLine < mBlockEndLines[lastBlock]; int numAddedBlocks = 0; if (createBlockBefore) numAddedBlocks++; @@ -863,12 +865,18 @@ public class DynamicLayout extends Layout if (numAddedBlocks + numRemovedBlocks != 0 && mBlocksAlwaysNeedToBeRedrawn != null) { final ArraySet set = new ArraySet<>(); + final int changedBlockCount = numAddedBlocks - numRemovedBlocks; for (int i = 0; i < mBlocksAlwaysNeedToBeRedrawn.size(); i++) { Integer block = mBlocksAlwaysNeedToBeRedrawn.valueAt(i); - if (block > firstBlock) { - block += numAddedBlocks - numRemovedBlocks; + if (block < firstBlock) { + // block index is before firstBlock add it since it did not change + set.add(block); + } + if (block > lastBlock) { + // block index is after lastBlock, the index reduced to += changedBlockCount + block += changedBlockCount; + set.add(block); } - set.add(block); } mBlocksAlwaysNeedToBeRedrawn = set; } diff --git a/core/tests/coretests/src/android/text/DynamicLayoutTest.java b/core/tests/coretests/src/android/text/DynamicLayoutTest.java index aa9aed8e2fcc6..ea954f65476d4 100644 --- a/core/tests/coretests/src/android/text/DynamicLayoutTest.java +++ b/core/tests/coretests/src/android/text/DynamicLayoutTest.java @@ -30,6 +30,7 @@ import android.platform.test.annotations.Presubmit; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.text.style.ReplacementSpan; +import android.util.ArraySet; import org.junit.Test; import org.junit.runner.RunWith; @@ -189,6 +190,27 @@ public class DynamicLayoutTest { layout.getLineExtra(100); } + @Test + public void testReflow_afterSpannableEdit() { + final String text = "a\nb:\uD83C\uDF1A c \n\uD83C\uDF1A"; + final int length = text.length(); + final SpannableStringBuilder spannable = new SpannableStringBuilder(text); + spannable.setSpan(new MockReplacementSpan(), 4, 6, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + spannable.setSpan(new MockReplacementSpan(), 10, length, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); + + final DynamicLayout layout = new DynamicLayout(spannable, new TextPaint(), WIDTH, + ALIGN_NORMAL, 1.0f /*spacingMultiplier*/, 0f /*spacingAdd*/, false /*includepad*/); + + spannable.delete(8, 9); + spannable.replace(7, 8, "ch"); + + layout.reflow(spannable, 0, length, length); + final ArraySet blocks = layout.getBlocksAlwaysNeedToBeRedrawn(); + for (Integer value : blocks) { + assertTrue("Block index should not be negative", value >= 0); + } + } + @Test public void testFallbackLineSpacing() { // All glyphs in the fonts are 1em wide.