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

Commit 626d3c22 authored by Victoria Lease's avatar Victoria Lease
Browse files

bidiFlags != SkPaint::Flags 

We've a number of native functions in the text layout path that take
a bidiFlags argument. We've a number of callers of those functions
passing in SkPaint::Flags in that slot. This completely breaks text
directionality for the affected functions, as
SkPaint::kAntiAlias_Flag happens to share values with kBidi_RTL,
resulting in anti-aliased SkPaints measuring text as if it were RTL,
and non-anti-aliased SkPaints measuring text as if it were LTR,
regardless of the actual text directionality. Oops!

To address the issue, this commit replaces erroneous calls to
SkPaint.getFlags() with the value of Paint.mBidiFlags, and includes
the necessary plumbing to get that value where it needs to be.

Bug: 8471481
Change-Id: I2d04b70defed3130fc1ad13f4c9098f5fce4ffde
parent 44bed6af

core/jni/android/graphics/Paint.cpp

+38 −38
Original line number Diff line number Diff line
@@ -383,7 +383,8 @@ public: 
        return descent - ascent + leading;

    }



    static jfloat measureText_CII(JNIEnv* env, jobject jpaint, jcharArray text, int index, int count) {

    static jfloat measureText_CIII(JNIEnv* env, jobject jpaint, jcharArray text, int index, int count,

            jint bidiFlags) {

        NPE_CHECK_RETURN_ZERO(env, jpaint);

        NPE_CHECK_RETURN_ZERO(env, text);
@@ -401,13 +402,14 @@ public: 
        jfloat result = 0;



        TextLayout::getTextRunAdvances(paint, textArray, index, count, textLength,

                paint->getFlags(), NULL /* dont need all advances */, &result);

                bidiFlags, NULL /* dont need all advances */, &result);



        env->ReleaseCharArrayElements(text, const_cast<jchar*>(textArray), JNI_ABORT);

        return result;

    }



    static jfloat measureText_StringII(JNIEnv* env, jobject jpaint, jstring text, int start, int end) {

    static jfloat measureText_StringIII(JNIEnv* env, jobject jpaint, jstring text, int start, int end,

            jint bidiFlags) {

        NPE_CHECK_RETURN_ZERO(env, jpaint);

        NPE_CHECK_RETURN_ZERO(env, text);
@@ -426,13 +428,13 @@ public: 
        jfloat width = 0;



        TextLayout::getTextRunAdvances(paint, textArray, start, count, textLength,

                paint->getFlags(), NULL /* dont need all advances */, &width);

                bidiFlags, NULL /* dont need all advances */, &width);



        env->ReleaseStringChars(text, textArray);

        return width;

    }



    static jfloat measureText_String(JNIEnv* env, jobject jpaint, jstring text) {

    static jfloat measureText_StringI(JNIEnv* env, jobject jpaint, jstring text, jint bidiFlags) {

        NPE_CHECK_RETURN_ZERO(env, jpaint);

        NPE_CHECK_RETURN_ZERO(env, text);
@@ -446,13 +448,14 @@ public: 
        jfloat width = 0;



        TextLayout::getTextRunAdvances(paint, textArray, 0, textLength, textLength,

                paint->getFlags(), NULL /* dont need all advances */, &width);

                bidiFlags, NULL /* dont need all advances */, &width);



        env->ReleaseStringChars(text, textArray);

        return width;

    }



    static int dotextwidths(JNIEnv* env, SkPaint* paint, const jchar text[], int count, jfloatArray widths) {

    static int dotextwidths(JNIEnv* env, SkPaint* paint, const jchar text[], int count, jfloatArray widths,

            jint bidiFlags) {

        NPE_CHECK_RETURN_ZERO(env, paint);

        NPE_CHECK_RETURN_ZERO(env, text);
@@ -473,23 +476,24 @@ public: 
        jfloat* widthsArray = autoWidths.ptr();



        TextLayout::getTextRunAdvances(paint, text, 0, count, count,

                paint->getFlags(), widthsArray, NULL /* dont need totalAdvance */);

                bidiFlags, widthsArray, NULL /* dont need totalAdvance */);



        return count;

    }



    static int getTextWidths___CII_F(JNIEnv* env, jobject clazz, SkPaint* paint, jcharArray text, int index, int count, jfloatArray widths) {

    static int getTextWidths___CIII_F(JNIEnv* env, jobject clazz, SkPaint* paint, jcharArray text,

            int index, int count, jint bidiFlags, jfloatArray widths) {

        const jchar* textArray = env->GetCharArrayElements(text, NULL);

        count = dotextwidths(env, paint, textArray + index, count, widths);

        count = dotextwidths(env, paint, textArray + index, count, widths, bidiFlags);

        env->ReleaseCharArrayElements(text, const_cast<jchar*>(textArray),

                                      JNI_ABORT);

        return count;

    }



    static int getTextWidths__StringII_F(JNIEnv* env, jobject clazz, SkPaint* paint, jstring text,

            int start, int end, jfloatArray widths) {

    static int getTextWidths__StringIII_F(JNIEnv* env, jobject clazz, SkPaint* paint, jstring text,

            int start, int end, jint bidiFlags, jfloatArray widths) {

        const jchar* textArray = env->GetStringChars(text, NULL);

        int count = dotextwidths(env, paint, textArray + start, end - start, widths);

        int count = dotextwidths(env, paint, textArray + start, end - start, widths, bidiFlags);

        env->ReleaseStringChars(text, textArray);

        return count;

    }
@@ -685,10 +689,10 @@ public: 
    }



    static int breakText(JNIEnv* env, SkPaint& paint, const jchar text[],

                         int count, float maxWidth, jfloatArray jmeasured,

                         int count, float maxWidth, jint bidiFlags, jfloatArray jmeasured,

                         SkPaint::TextBufferDirection tbd) {

        sp<TextLayoutValue> value = TextLayoutEngine::getInstance().getValue(&paint,

                text, 0, count, count, paint.getFlags());

                text, 0, count, count, bidiFlags);

        if (value == NULL) {

            return 0;

        }
@@ -706,7 +710,7 @@ public: 
    }



    static int breakTextC(JNIEnv* env, jobject jpaint, jcharArray jtext,

            int index, int count, float maxWidth, jfloatArray jmeasuredWidth) {

            int index, int count, float maxWidth, jint bidiFlags, jfloatArray jmeasuredWidth) {

        NPE_CHECK_RETURN_ZERO(env, jpaint);

        NPE_CHECK_RETURN_ZERO(env, jtext);
@@ -727,14 +731,14 @@ public: 
        SkPaint*     paint = GraphicsJNI::getNativePaint(env, jpaint);

        const jchar* text = env->GetCharArrayElements(jtext, NULL);

        count = breakText(env, *paint, text + index, count, maxWidth,

                          jmeasuredWidth, tbd);

                          bidiFlags, jmeasuredWidth, tbd);

        env->ReleaseCharArrayElements(jtext, const_cast<jchar*>(text),

                                      JNI_ABORT);

        return count;

    }



    static int breakTextS(JNIEnv* env, jobject jpaint, jstring jtext,

                bool forwards, float maxWidth, jfloatArray jmeasuredWidth) {

                bool forwards, float maxWidth, jint bidiFlags, jfloatArray jmeasuredWidth) {

        NPE_CHECK_RETURN_ZERO(env, jpaint);

        NPE_CHECK_RETURN_ZERO(env, jtext);
@@ -745,22 +749,20 @@ public: 
        SkPaint* paint = GraphicsJNI::getNativePaint(env, jpaint);

        int count = env->GetStringLength(jtext);

        const jchar* text = env->GetStringChars(jtext, NULL);

        count = breakText(env, *paint, text, count, maxWidth,

                          jmeasuredWidth, tbd);

        count = breakText(env, *paint, text, count, maxWidth, bidiFlags, jmeasuredWidth, tbd);

        env->ReleaseStringChars(jtext, text);

        return count;

    }



    static void doTextBounds(JNIEnv* env, const jchar* text, int count,

                             jobject bounds, const SkPaint& paint)

    {

                             jobject bounds, const SkPaint& paint, jint bidiFlags) {

        SkRect  r;

        r.set(0,0,0,0);



        SkIRect ir;



        sp<TextLayoutValue> value = TextLayoutEngine::getInstance().getValue(&paint,

                text, 0, count, count, paint.getFlags());

                text, 0, count, count, bidiFlags);

        if (value == NULL) {

            return;

        }
@@ -770,18 +772,16 @@ public: 
    }



    static void getStringBounds(JNIEnv* env, jobject, const SkPaint* paint,

                                jstring text, int start, int end, jobject bounds)

    {

                                jstring text, int start, int end, jint bidiFlags, jobject bounds) {

        const jchar* textArray = env->GetStringChars(text, NULL);

        doTextBounds(env, textArray + start, end - start, bounds, *paint);

        doTextBounds(env, textArray + start, end - start, bounds, *paint, bidiFlags);

        env->ReleaseStringChars(text, textArray);

    }



    static void getCharArrayBounds(JNIEnv* env, jobject, const SkPaint* paint,

                        jcharArray text, int index, int count, jobject bounds)

    {

                        jcharArray text, int index, int count, jint bidiFlags, jobject bounds) {

        const jchar* textArray = env->GetCharArrayElements(text, NULL);

        doTextBounds(env, textArray + index, count, bounds, *paint);

        doTextBounds(env, textArray + index, count, bounds, *paint, bidiFlags);

        env->ReleaseCharArrayElements(text, const_cast<jchar*>(textArray),

                                      JNI_ABORT);

    }
@@ -841,13 +841,13 @@ static JNINativeMethod methods[] = { 
    {"descent","()F", (void*) SkPaintGlue::descent},

    {"getFontMetrics", "(Landroid/graphics/Paint$FontMetrics;)F", (void*)SkPaintGlue::getFontMetrics},

    {"getFontMetricsInt", "(Landroid/graphics/Paint$FontMetricsInt;)I", (void*)SkPaintGlue::getFontMetricsInt},

    {"native_measureText","([CII)F", (void*) SkPaintGlue::measureText_CII},

    {"native_measureText","(Ljava/lang/String;)F", (void*) SkPaintGlue::measureText_String},

    {"native_measureText","(Ljava/lang/String;II)F", (void*) SkPaintGlue::measureText_StringII},

    {"native_breakText","([CIIF[F)I", (void*) SkPaintGlue::breakTextC},

    {"native_breakText","(Ljava/lang/String;ZF[F)I", (void*) SkPaintGlue::breakTextS},

    {"native_getTextWidths","(I[CII[F)I", (void*) SkPaintGlue::getTextWidths___CII_F},

    {"native_getTextWidths","(ILjava/lang/String;II[F)I", (void*) SkPaintGlue::getTextWidths__StringII_F},

    {"native_measureText","([CIII)F", (void*) SkPaintGlue::measureText_CIII},

    {"native_measureText","(Ljava/lang/String;I)F", (void*) SkPaintGlue::measureText_StringI},

    {"native_measureText","(Ljava/lang/String;III)F", (void*) SkPaintGlue::measureText_StringIII},

    {"native_breakText","([CIIFI[F)I", (void*) SkPaintGlue::breakTextC},

    {"native_breakText","(Ljava/lang/String;ZFI[F)I", (void*) SkPaintGlue::breakTextS},

    {"native_getTextWidths","(I[CIII[F)I", (void*) SkPaintGlue::getTextWidths___CIII_F},

    {"native_getTextWidths","(ILjava/lang/String;III[F)I", (void*) SkPaintGlue::getTextWidths__StringIII_F},

    {"native_getTextRunAdvances","(I[CIIIII[FI)F",

        (void*) SkPaintGlue::getTextRunAdvances___CIIIII_FI},

    {"native_getTextRunAdvances","(ILjava/lang/String;IIIII[FI)F",
@@ -861,9 +861,9 @@ static JNINativeMethod methods[] = { 
        (void*) SkPaintGlue::getTextRunCursor__String},

    {"native_getTextPath","(II[CIIFFI)V", (void*) SkPaintGlue::getTextPath___C},

    {"native_getTextPath","(IILjava/lang/String;IIFFI)V", (void*) SkPaintGlue::getTextPath__String},

    {"nativeGetStringBounds", "(ILjava/lang/String;IILandroid/graphics/Rect;)V",

    {"nativeGetStringBounds", "(ILjava/lang/String;IIILandroid/graphics/Rect;)V",

                                        (void*) SkPaintGlue::getStringBounds },

    {"nativeGetCharArrayBounds", "(I[CIILandroid/graphics/Rect;)V",

    {"nativeGetCharArrayBounds", "(I[CIIILandroid/graphics/Rect;)V",

                                    (void*) SkPaintGlue::getCharArrayBounds },

    {"nSetShadowLayer", "(FFFI)V", (void*)SkPaintGlue::setShadowLayer}

};

graphics/java/android/graphics/Paint.java

+25 −25
Original line number Diff line number Diff line
@@ -1295,17 +1295,17 @@ public class Paint { 
            return 0f;

        }

        if (!mHasCompatScaling) {

            return (float) Math.ceil(native_measureText(text, index, count));

            return (float) Math.ceil(native_measureText(text, index, count, mBidiFlags));

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        float w = native_measureText(text, index, count);

        float w = native_measureText(text, index, count, mBidiFlags);

        setTextSize(oldSize);

        return (float) Math.ceil(w*mInvCompatScaling);

    }



    private native float native_measureText(char[] text, int index, int count);

    private native float native_measureText(char[] text, int index, int count, int bidiFlags);

    

    /**

     * Return the width of the text.
@@ -1327,17 +1327,17 @@ public class Paint { 
            return 0f;

        }

        if (!mHasCompatScaling) {

            return (float) Math.ceil(native_measureText(text, start, end));

            return (float) Math.ceil(native_measureText(text, start, end, mBidiFlags));

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        float w = native_measureText(text, start, end);

        float w = native_measureText(text, start, end, mBidiFlags);

        setTextSize(oldSize);

        return (float) Math.ceil(w*mInvCompatScaling);

    }



    private native float native_measureText(String text, int start, int end);

    private native float native_measureText(String text, int start, int end, int bidiFlags);

    

    /**

     * Return the width of the text.
@@ -1355,16 +1355,16 @@ public class Paint { 
        }



        if (!mHasCompatScaling) {

            return (float) Math.ceil(native_measureText(text));

            return (float) Math.ceil(native_measureText(text, mBidiFlags));

        }

        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        float w = native_measureText(text);

        float w = native_measureText(text, mBidiFlags);

        setTextSize(oldSize);

        return (float) Math.ceil(w*mInvCompatScaling);

    }



    private native float native_measureText(String text);

    private native float native_measureText(String text, int bidiFlags);

    

    /**

     * Return the width of the text.
@@ -1431,12 +1431,12 @@ public class Paint { 
            return 0;

        }

        if (!mHasCompatScaling) {

            return native_breakText(text, index, count, maxWidth, measuredWidth);

            return native_breakText(text, index, count, maxWidth, mBidiFlags, measuredWidth);

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        int res = native_breakText(text, index, count, maxWidth*mCompatScaling,

        int res = native_breakText(text, index, count, maxWidth*mCompatScaling, mBidiFlags,

                measuredWidth);

        setTextSize(oldSize);

        if (measuredWidth != null) measuredWidth[0] *= mInvCompatScaling;
@@ -1444,7 +1444,7 @@ public class Paint { 
    }



    private native int native_breakText(char[] text, int index, int count,

                                        float maxWidth, float[] measuredWidth);

                                        float maxWidth, int bidiFlags, float[] measuredWidth);



    /**

     * Measure the text, stopping early if the measured width exceeds maxWidth.
@@ -1521,12 +1521,12 @@ public class Paint { 
            return 0;

        }

        if (!mHasCompatScaling) {

            return native_breakText(text, measureForwards, maxWidth, measuredWidth);

            return native_breakText(text, measureForwards, maxWidth, mBidiFlags, measuredWidth);

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        int res = native_breakText(text, measureForwards, maxWidth*mCompatScaling,

        int res = native_breakText(text, measureForwards, maxWidth*mCompatScaling, mBidiFlags,

                measuredWidth);

        setTextSize(oldSize);

        if (measuredWidth != null) measuredWidth[0] *= mInvCompatScaling;
@@ -1534,7 +1534,7 @@ public class Paint { 
    }



    private native int native_breakText(String text, boolean measureForwards,

                                        float maxWidth, float[] measuredWidth);

                                        float maxWidth, int bidiFlags, float[] measuredWidth);



    /**

     * Return the advance widths for the characters in the string.
@@ -1560,12 +1560,12 @@ public class Paint { 
            return 0;

        }

        if (!mHasCompatScaling) {

            return native_getTextWidths(mNativePaint, text, index, count, widths);

            return native_getTextWidths(mNativePaint, text, index, count, mBidiFlags, widths);

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        int res = native_getTextWidths(mNativePaint, text, index, count, widths);

        int res = native_getTextWidths(mNativePaint, text, index, count, mBidiFlags, widths);

        setTextSize(oldSize);

        for (int i=0; i<res; i++) {

            widths[i] *= mInvCompatScaling;
@@ -1642,12 +1642,12 @@ public class Paint { 
            return 0;

        }

        if (!mHasCompatScaling) {

            return native_getTextWidths(mNativePaint, text, start, end, widths);

            return native_getTextWidths(mNativePaint, text, start, end, mBidiFlags, widths);

        }



        final float oldSize = getTextSize();

        setTextSize(oldSize*mCompatScaling);

        int res = native_getTextWidths(mNativePaint, text, start, end, widths);

        int res = native_getTextWidths(mNativePaint, text, start, end, mBidiFlags, widths);

        setTextSize(oldSize);

        for (int i=0; i<res; i++) {

            widths[i] *= mInvCompatScaling;
@@ -2073,7 +2073,7 @@ public class Paint { 
        if (bounds == null) {

            throw new NullPointerException("need bounds Rect");

        }

        nativeGetStringBounds(mNativePaint, text, start, end, bounds);

        nativeGetStringBounds(mNativePaint, text, start, end, mBidiFlags, bounds);

    }

    

    /**
@@ -2093,7 +2093,7 @@ public class Paint { 
        if (bounds == null) {

            throw new NullPointerException("need bounds Rect");

        }

        nativeGetCharArrayBounds(mNativePaint, text, index, count, bounds);

        nativeGetCharArrayBounds(mNativePaint, text, index, count, mBidiFlags, bounds);

    }

    

    @Override
@@ -2140,9 +2140,9 @@ public class Paint { 
                                                    String locale);



    private static native int native_getTextWidths(int native_object,

                            char[] text, int index, int count, float[] widths);

                            char[] text, int index, int count, int bidiFlags, float[] widths);

    private static native int native_getTextWidths(int native_object,

                            String text, int start, int end, float[] widths);

                            String text, int start, int end, int bidiFlags, float[] widths);



    private static native int native_getTextGlyphs(int native_object,

            String text, int start, int end, int contextStart, int contextEnd,
@@ -2165,8 +2165,8 @@ public class Paint { 
    private static native void native_getTextPath(int native_object, int bidiFlags,

                String text, int start, int end, float x, float y, int path);

    private static native void nativeGetStringBounds(int nativePaint,

                                String text, int start, int end, Rect bounds);

                                String text, int start, int end, int bidiFlags, Rect bounds);

    private static native void nativeGetCharArrayBounds(int nativePaint,

                                char[] text, int index, int count, Rect bounds);

                                char[] text, int index, int count, int bidiFlags, Rect bounds);

    private static native void finalizer(int nativePaint);

}