Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3630d086 authored by Roozbeh Pournader's avatar Roozbeh Pournader
Browse files

Fix space counting for justified text 

Previously, the number of stretchable spaces in a line were
miscounted, always returning one more than the actual number. We now
count them correctly.

Also, disable NBSP as an stretchable space, since in some use cases
it should not stretch and the implementation in Minikin was broken
anyway.

Test: Manual (justified text is now properly aligned on right side)
Test: bit FrameworksCoreTests:android.text.TextLineTest
Fixes: 68009059
Bug: 68204709
Change-Id: I473415a2a51eec75377d6ffc4a9adddc2033d360
parent a5da2cf5

core/java/android/text/TextLine.java

+20 −18
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import android.text.style.MetricAffectingSpan; 
import android.text.style.ReplacementSpan;

import android.util.Log;



import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.ArrayUtils;



import java.util.ArrayList;
@@ -44,7 +45,8 @@ import java.util.ArrayList; 
 *

 * @hide

 */

class TextLine {

@VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

public class TextLine {

    private static final boolean DEBUG = false;



    private TextPaint mPaint;
@@ -82,7 +84,8 @@ class TextLine { 
     *

     * @return an uninitialized TextLine

     */

    static TextLine obtain() {

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    public static TextLine obtain() {

        TextLine tl;

        synchronized (sCached) {

            for (int i = sCached.length; --i >= 0;) {
@@ -107,7 +110,8 @@ class TextLine { 
     * @return null, as a convenience from clearing references to the provided

     * TextLine

     */

    static TextLine recycle(TextLine tl) {

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    public static TextLine recycle(TextLine tl) {

        tl.mText = null;

        tl.mPaint = null;

        tl.mDirections = null;
@@ -142,7 +146,8 @@ class TextLine { 
     * @param hasTabs true if the line might contain tabs

     * @param tabStops the tabStops. Can be null.

     */

    void set(TextPaint paint, CharSequence text, int start, int limit, int dir,

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    public void set(TextPaint paint, CharSequence text, int start, int limit, int dir,

            Directions directions, boolean hasTabs, TabStops tabStops) {

        mPaint = paint;

        mText = text;
@@ -196,7 +201,8 @@ class TextLine { 
    /**

     * Justify the line to the given width.

     */

    void justify(float justifyWidth) {

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    public void justify(float justifyWidth) {

        int end = mLen;

        while (end > 0 && isLineEndSpace(mText.charAt(mStart + end - 1))) {

            end--;
@@ -277,7 +283,8 @@ class TextLine { 
     * @param fmi receives font metrics information, can be null

     * @return the signed width of the line

     */

    float metrics(FontMetricsInt fmi) {

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    public float metrics(FontMetricsInt fmi) {

        return measure(mLen, false, fmi);

    }
@@ -1165,24 +1172,19 @@ class TextLine { 
    }



    private boolean isStretchableWhitespace(int ch) {

        // TODO: Support other stretchable whitespace. (Bug: 34013491)

        return ch == 0x0020 || ch == 0x00A0;

    }



    private int nextStretchableSpace(int start, int end) {

        for (int i = start; i < end; i++) {

            final char c = mCharsValid ? mChars[i] : mText.charAt(i + mStart);

            if (isStretchableWhitespace(c)) return i;

        }

        return end;

        // TODO: Support NBSP and other stretchable whitespace (b/34013491 and b/68204709).

        return ch == 0x0020;

    }



    /* Return the number of spaces in the text line, for the purpose of justification */

    private int countStretchableSpaces(int start, int end) {

        int count = 0;

        for (int i = start; i < end; i = nextStretchableSpace(i + 1, end)) {

        for (int i = start; i < end; i++) {

            final char c = mCharsValid ? mChars[i] : mText.charAt(i + mStart);

            if (isStretchableWhitespace(c)) {

                count++;

            }

        }

        return count;

    }

core/tests/coretests/src/android/text/TextLineTest.java

0 → 100644
+67 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2017 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package android.text;



import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;



import android.support.test.filters.SmallTest;

import android.support.test.filters.Suppress;

import android.support.test.runner.AndroidJUnit4;



import org.junit.Test;

import org.junit.runner.RunWith;



@SmallTest

@RunWith(AndroidJUnit4.class)

public class TextLineTest {

    private boolean stretchesToFullWidth(CharSequence line) {

        final TextPaint paint = new TextPaint();

        final TextLine tl = TextLine.obtain();

        tl.set(paint, line, 0, line.length(), Layout.DIR_LEFT_TO_RIGHT,

                Layout.DIRS_ALL_LEFT_TO_RIGHT, false /* hasTabs */, null /* tabStops */);

        final float originalWidth = tl.metrics(null);

        final float expandedWidth = 2 * originalWidth;



        tl.justify(expandedWidth);

        final float newWidth = tl.metrics(null);

        TextLine.recycle(tl);

        return Math.abs(newWidth - expandedWidth) < 0.5;

    }



    @Test

    public void testJustify_spaces() {

        // There are no spaces to stretch.

        assertFalse(stretchesToFullWidth("text"));



        assertTrue(stretchesToFullWidth("one space"));

        assertTrue(stretchesToFullWidth("exactly two spaces"));

        assertTrue(stretchesToFullWidth("up to three spaces"));

    }



    // NBSP should also stretch when it's not used as a base for a combining mark. This doesn't work

    // yet (b/68204709).

    @Suppress

    public void disabledTestJustify_NBSP() {

        final char nbsp = '\u00A0';

        assertTrue(stretchesToFullWidth("non-breaking" + nbsp + "space"));

        assertTrue(stretchesToFullWidth("mix" + nbsp + "and match"));



        final char combining_acute = '\u0301';

        assertFalse(stretchesToFullWidth("combining" + nbsp + combining_acute + "acute"));

    }

}