From de41eeaafd1c42525b8dc66b105af10fad5e2296 Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Thu, 7 Mar 2019 16:19:05 -0800 Subject: Address API review from the council Here are the list this CL includes: - Adding missing @NonNull, @Nullable, @IntRange annotations - Added @Px annotation to Paint.set/getWordSpacing - Change Builder classes to final. - Change FileDescriptor to ParcelFileDescriptor as described in guideline. - With this change, additional error handling is required in old Typeface.Builder but we can keep the same behavior (silently fallback to system fallback in error case). - Fix comment in getLineHyphenEdit in LineBreaker.Result - Rename setJustified to setJustificationMode in LineBreaker.Builder Fix: 126702154 Fix: 126701272 Fix: 126701569 Fix: 126701953 Fix: 126701507 Fix: 126701667 Fix: 126700866 Fix: 126701009 Fix: 126700795 Fix: 126700292 Fix: 126699209 Fix: 124795407 Fix: 124795470 Bug: 124794844 Bug: 124794899 Test: m update-api Test: TreeHugger Change-Id: If6e8566b0eef9151d35d75b7f258128d7d3df358 --- graphics/java/android/graphics/Paint.java | 11 ++++++--- graphics/java/android/graphics/Typeface.java | 27 ++++++++++++++++------ graphics/java/android/graphics/fonts/Font.java | 12 +++++----- .../java/android/graphics/fonts/FontFamily.java | 6 ++--- .../java/android/graphics/text/LineBreaker.java | 24 ++++++++++--------- .../java/android/graphics/text/MeasuredText.java | 14 +++++------ 6 files changed, 57 insertions(+), 37 deletions(-) (limited to 'graphics/java/android') diff --git a/graphics/java/android/graphics/Paint.java b/graphics/java/android/graphics/Paint.java index e617c42cc70d..9eeb43b579bc 100644 --- a/graphics/java/android/graphics/Paint.java +++ b/graphics/java/android/graphics/Paint.java @@ -1733,7 +1733,7 @@ public class Paint { * @return the paint's extra word-spacing for drawing text in pixels. * @see #setWordSpacing(float) */ - public float getWordSpacing() { + public @Px float getWordSpacing() { return nGetWordSpacing(mNativePaint); } @@ -1746,7 +1746,7 @@ public class Paint { * @param wordSpacing set the paint's extra word-spacing for drawing text in pixels. * @see #getWordSpacing() */ - public void setWordSpacing(float wordSpacing) { + public void setWordSpacing(@Px float wordSpacing) { nSetWordSpacing(mNativePaint, wordSpacing); } @@ -2706,6 +2706,8 @@ public class Paint { } /** + * Retrieve the text boundary box and store to bounds. + * * Return in bounds (allocated by the caller) the smallest rectangle that * encloses all of the characters, with an implied origin at (0,0). * @@ -2725,6 +2727,8 @@ public class Paint { } /** + * Retrieve the text boundary box and store to bounds. + * * Return in bounds (allocated by the caller) the smallest rectangle that * encloses all of the characters, with an implied origin at (0,0). * @@ -2736,7 +2740,8 @@ public class Paint { * @param end 1 past the last char in the text to measure * @param bounds returns the unioned bounds of all the text. Must be allocated by the caller */ - public void getTextBounds(CharSequence text, int start, int end, Rect bounds) { + public void getTextBounds(@NonNull CharSequence text, int start, int end, + @NonNull Rect bounds) { if ((start | end | (end - start) | (text.length() - end)) < 0) { throw new IndexOutOfBoundsException(); } diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java index e93e757410e0..95187149b985 100644 --- a/graphics/java/android/graphics/Typeface.java +++ b/graphics/java/android/graphics/Typeface.java @@ -34,6 +34,7 @@ import android.graphics.fonts.FontVariationAxis; import android.graphics.fonts.SystemFonts; import android.os.Build; import android.os.Build.VERSION_CODES; +import android.os.ParcelFileDescriptor; import android.provider.FontRequest; import android.provider.FontsContract; import android.text.FontConfig; @@ -368,7 +369,7 @@ public class Typeface { private final AssetManager mAssetManager; private final String mPath; - private final Font.Builder mFontBuilder; + private final @Nullable Font.Builder mFontBuilder; private String mFallbackFamilyName; @@ -395,7 +396,16 @@ public class Typeface { * @param fd The file descriptor. The passed fd must be mmap-able. */ public Builder(@NonNull FileDescriptor fd) { - mFontBuilder = new Font.Builder(fd); + Font.Builder builder; + try { + builder = new Font.Builder(ParcelFileDescriptor.dup(fd)); + } catch (IOException e) { + // We cannot tell the error to developer at this moment since we cannot change the + // public API signature. Instead, silently fallbacks to system fallback in the build + // method as the same as other error cases. + builder = null; + } + mFontBuilder = builder; mAssetManager = null; mPath = null; } @@ -585,6 +595,9 @@ public class Typeface { * @return Newly created Typeface. May return null if some parameters are invalid. */ public Typeface build() { + if (mFontBuilder == null) { + return resolveFallbackTypeface(); + } try { final Font font = mFontBuilder.build(); final String key = mAssetManager == null ? null : createAssetUid( @@ -687,7 +700,7 @@ public class Typeface { * *

*/ - public static class CustomFallbackBuilder { + public static final class CustomFallbackBuilder { private static final int MAX_CUSTOM_FALLBACK = 64; private final ArrayList mFamilies = new ArrayList<>(); private String mFallbackName = null; @@ -728,7 +741,7 @@ public class Typeface { * @param familyName a family name to be used for fallback if the provided fonts can not be * used */ - public CustomFallbackBuilder setSystemFallback(@NonNull String familyName) { + public @NonNull CustomFallbackBuilder setSystemFallback(@NonNull String familyName) { Preconditions.checkNotNull(familyName); mFallbackName = familyName; return this; @@ -743,7 +756,7 @@ public class Typeface { * * @param style a font style */ - public CustomFallbackBuilder setStyle(@NonNull FontStyle style) { + public @NonNull CustomFallbackBuilder setStyle(@NonNull FontStyle style) { mStyle = style; return this; } @@ -758,7 +771,7 @@ public class Typeface { * @param family a fallback family * @throws IllegalArgumentException if you give more than 64 custom fallback families */ - public CustomFallbackBuilder addCustomFallback(@NonNull FontFamily family) { + public @NonNull CustomFallbackBuilder addCustomFallback(@NonNull FontFamily family) { Preconditions.checkNotNull(family); Preconditions.checkArgument(mFamilies.size() < getMaxCustomFallbackCount(), "Custom fallback limit exceeded(" + getMaxCustomFallbackCount() + ")"); @@ -771,7 +784,7 @@ public class Typeface { * * @return the Typeface object */ - public Typeface build() { + public @NonNull Typeface build() { final int userFallbackSize = mFamilies.size(); final FontFamily[] fallback = SystemFonts.getSystemFallback(mFallbackName); final long[] ptrArray = new long[fallback.length + userFallbackSize]; diff --git a/graphics/java/android/graphics/fonts/Font.java b/graphics/java/android/graphics/fonts/Font.java index 7f165bfc6a7d..f715f43344d3 100644 --- a/graphics/java/android/graphics/fonts/Font.java +++ b/graphics/java/android/graphics/fonts/Font.java @@ -22,6 +22,7 @@ import android.annotation.Nullable; import android.content.res.AssetManager; import android.content.res.Resources; import android.os.LocaleList; +import android.os.ParcelFileDescriptor; import android.util.TypedValue; import com.android.internal.util.Preconditions; @@ -31,7 +32,6 @@ import dalvik.annotation.optimization.CriticalNative; import libcore.util.NativeAllocationRegistry; import java.io.File; -import java.io.FileDescriptor; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; @@ -53,7 +53,7 @@ public final class Font { /** * A builder class for creating new Font. */ - public static class Builder { + public static final class Builder { private static final NativeAllocationRegistry sAssetByteBufferRegistroy = new NativeAllocationRegistry(ByteBuffer.class.getClassLoader(), nGetReleaseNativeAssetFunc(), 64); @@ -122,7 +122,7 @@ public final class Font { * * @param fd a file descriptor */ - public Builder(@NonNull FileDescriptor fd) { + public Builder(@NonNull ParcelFileDescriptor fd) { this(fd, 0, -1); } @@ -133,9 +133,9 @@ public final class Font { * @param offset an offset to of the font data in the file * @param size a size of the font data. If -1 is passed, use until end of the file. */ - public Builder(@NonNull FileDescriptor fd, @IntRange(from = 0) long offset, + public Builder(@NonNull ParcelFileDescriptor fd, @IntRange(from = 0) long offset, @IntRange(from = -1) long size) { - try (FileInputStream fis = new FileInputStream(fd)) { + try (FileInputStream fis = new FileInputStream(fd.getFileDescriptor())) { final FileChannel fc = fis.getChannel(); size = (size == -1) ? fc.size() - offset : size; mBuffer = fc.map(FileChannel.MapMode.READ_ONLY, offset, size); @@ -467,7 +467,7 @@ public final class Font { * @see Builder#setSlant(int) * @return a font style */ - public FontStyle getStyle() { + public @NonNull FontStyle getStyle() { return mFontStyle; } diff --git a/graphics/java/android/graphics/fonts/FontFamily.java b/graphics/java/android/graphics/fonts/FontFamily.java index c0f1b163ea11..4772c1cff7eb 100644 --- a/graphics/java/android/graphics/fonts/FontFamily.java +++ b/graphics/java/android/graphics/fonts/FontFamily.java @@ -61,7 +61,7 @@ public final class FontFamily { /** * A builder class for creating new FontFamily. */ - public static class Builder { + public static final class Builder { private static final NativeAllocationRegistry sFamilyRegistory = new NativeAllocationRegistry(FontFamily.class.getClassLoader(), nGetReleaseNativeFamily(), 64); @@ -152,7 +152,7 @@ public final class FontFamily { * @param index an index of the font * @return a registered font */ - public Font getFont(@IntRange(from = 0) int index) { + public @NonNull Font getFont(@IntRange(from = 0) int index) { return mFonts.get(index); } @@ -161,7 +161,7 @@ public final class FontFamily { * * @return the number of fonts registered in this family. */ - public int getSize() { + public @IntRange(from = 1) int getSize() { return mFonts.size(); } diff --git a/graphics/java/android/graphics/text/LineBreaker.java b/graphics/java/android/graphics/text/LineBreaker.java index 16479095a63a..046bbcffb76a 100644 --- a/graphics/java/android/graphics/text/LineBreaker.java +++ b/graphics/java/android/graphics/text/LineBreaker.java @@ -177,10 +177,10 @@ public class LineBreaker { /** * Helper class for creating a {@link LineBreaker}. */ - public static class Builder { + public static final class Builder { private @BreakStrategy int mBreakStrategy = BREAK_STRATEGY_SIMPLE; private @HyphenationFrequency int mHyphenationFrequency = HYPHENATION_FREQUENCY_NONE; - private @JustificationMode int mJustified = JUSTIFICATION_MODE_NONE; + private @JustificationMode int mJustificationMode = JUSTIFICATION_MODE_NONE; private @Nullable int[] mIndents = null; /** @@ -189,7 +189,7 @@ public class LineBreaker { * You can change the line breaking behavior by setting break strategy. The default value is * {@link #BREAK_STRATEGY_SIMPLE}. */ - public Builder setBreakStrategy(@BreakStrategy int breakStrategy) { + public @NonNull Builder setBreakStrategy(@BreakStrategy int breakStrategy) { mBreakStrategy = breakStrategy; return this; } @@ -200,7 +200,8 @@ public class LineBreaker { * You can change the amount of automatic hyphenation used. The default value is * {@link #HYPHENATION_FREQUENCY_NONE}. */ - public Builder setHyphenationFrequency(@HyphenationFrequency int hyphenationFrequency) { + public @NonNull Builder setHyphenationFrequency( + @HyphenationFrequency int hyphenationFrequency) { mHyphenationFrequency = hyphenationFrequency; return this; } @@ -212,8 +213,8 @@ public class LineBreaker { * internal parameters for justification. * The default value is {@link #JUSTIFICATION_MODE_NONE} */ - public Builder setJustified(@JustificationMode int justified) { - mJustified = justified; + public @NonNull Builder setJustificationMode(@JustificationMode int justificationMode) { + mJustificationMode = justificationMode; return this; } @@ -224,7 +225,7 @@ public class LineBreaker { * amount is the sum of both left and right indentations. For lines past the last element in * the array, the indentation amount of the last element is used. */ - public Builder setIndents(@Nullable int[] indents) { + public @NonNull Builder setIndents(@Nullable int[] indents) { mIndents = indents; return this; } @@ -234,8 +235,9 @@ public class LineBreaker { * * You can reuse the Builder instance even after calling this method. */ - public LineBreaker build() { - return new LineBreaker(mBreakStrategy, mHyphenationFrequency, mJustified, mIndents); + public @NonNull LineBreaker build() { + return new LineBreaker(mBreakStrategy, mHyphenationFrequency, mJustificationMode, + mIndents); } } @@ -412,7 +414,7 @@ public class LineBreaker { } /** - * Returns a packed packed hyphen edit for the line. + * Returns a packed hyphen edit for the line. * * @param lineIndex an index of the line. * @return a packed hyphen edit for the line. @@ -451,7 +453,7 @@ public class LineBreaker { * @param constraints for a single paragraph * @param lineNumber a line number of this paragraph */ - public Result computeLineBreaks( + public @NonNull Result computeLineBreaks( @NonNull MeasuredText measuredPara, @NonNull ParagraphConstraints constraints, @IntRange(from = 0) int lineNumber) { diff --git a/graphics/java/android/graphics/text/MeasuredText.java b/graphics/java/android/graphics/text/MeasuredText.java index 2536619bb7c8..480aff289dfc 100644 --- a/graphics/java/android/graphics/text/MeasuredText.java +++ b/graphics/java/android/graphics/text/MeasuredText.java @@ -168,7 +168,7 @@ public class MeasuredText { * * Note: The appendStyle and appendReplacementRun should be called to cover the text length. */ - public static class Builder { + public static final class Builder { private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( MeasuredText.class.getClassLoader(), nGetReleaseFunc(), 1024); @@ -227,7 +227,7 @@ public class MeasuredText { * text * @param isRtl true if the text is in RTL context, otherwise false. */ - public Builder appendStyleRun(@NonNull Paint paint, @IntRange(from = 0) int length, + public @NonNull Builder appendStyleRun(@NonNull Paint paint, @IntRange(from = 0) int length, boolean isRtl) { Preconditions.checkNotNull(paint); Preconditions.checkArgument(length > 0, "length can not be negative"); @@ -253,8 +253,8 @@ public class MeasuredText { * text * @param width a replacement width of the range */ - public Builder appendReplacementRun(@NonNull Paint paint, - @IntRange(from = 0) int length, @FloatRange(from = 0) float width) { + public @NonNull Builder appendReplacementRun(@NonNull Paint paint, + @IntRange(from = 0) int length, @Px @FloatRange(from = 0) float width) { Preconditions.checkArgument(length > 0, "length can not be negative"); final int end = mCurrentOffset + length; Preconditions.checkArgument(end <= mText.length, "Replacement exceeds the text length"); @@ -275,7 +275,7 @@ public class MeasuredText { * * @param computeHyphenation true if you want to use automatic hyphenations. */ - public Builder setComputeHyphenation(boolean computeHyphenation) { + public @NonNull Builder setComputeHyphenation(boolean computeHyphenation) { mComputeHyphenation = computeHyphenation; return this; } @@ -292,7 +292,7 @@ public class MeasuredText { * * @param computeLayout true if you want to retrieve full layout info, e.g. bbox. */ - public Builder setComputeLayout(boolean computeLayout) { + public @NonNull Builder setComputeLayout(boolean computeLayout) { mComputeLayout = computeLayout; return this; } @@ -305,7 +305,7 @@ public class MeasuredText { * @throws IllegalStateException if the whole text is not covered by one or more runs (style * or replacement) */ - public MeasuredText build() { + public @NonNull MeasuredText build() { ensureNativePtrNoReuse(); if (mCurrentOffset != mText.length) { throw new IllegalStateException("Style info has not been provided for all text."); -- cgit v1.2.3-59-g8ed1b