Use -Werror in frameworks/minikin
am: 0815c28680
Change-Id: I2cad2ae2533df6023e4194c59edaee4fdf82eea0
diff --git a/libs/minikin/CmapCoverage.cpp b/libs/minikin/CmapCoverage.cpp
index 9659d0c..c56d07c 100644
--- a/libs/minikin/CmapCoverage.cpp
+++ b/libs/minikin/CmapCoverage.cpp
@@ -49,15 +49,25 @@
((uint32_t)data[offset + 2]) << 8 | ((uint32_t)data[offset + 3]);
}
-static void addRange(vector<uint32_t> &coverage, uint32_t start, uint32_t end) {
+// The start must be larger than or equal to coverage.back() if coverage is not empty.
+// Returns true if the range is appended. Otherwise returns false as an error.
+static bool addRange(vector<uint32_t> &coverage, uint32_t start, uint32_t end) {
#ifdef VERBOSE_DEBUG
ALOGD("adding range %d-%d\n", start, end);
#endif
if (coverage.empty() || coverage.back() < start) {
coverage.push_back(start);
coverage.push_back(end);
- } else {
+ return true;
+ } else if (coverage.back() == start) {
coverage.back() = end;
+ return true;
+ } else {
+ // Reject unordered range input since SparseBitSet assumes that the given range vector is
+ // sorted. OpenType specification says cmap entries are sorted in order of code point
+ // values, thus for OpenType compliant font files, we don't reach here.
+ android_errorWriteLog(0x534e4554, "32178311");
+ return false;
}
}
@@ -109,7 +119,7 @@
// No ranges left in rRanges. Just put all remaining ranges in lRanges.
do {
Range r = getRange(lRanges, li);
- addRange(out, r.start, r.end);
+ addRange(out, r.start, r.end); // Input is sorted. Never returns false.
li += 2;
} while (li < lsize);
break;
@@ -117,17 +127,17 @@
// No ranges left in lRanges. Just put all remaining ranges in rRanges.
do {
Range r = getRange(rRanges, ri);
- addRange(out, r.start, r.end);
+ addRange(out, r.start, r.end); // Input is sorted. Never returns false.
ri += 2;
} while (ri < rsize);
break;
} else if (!Range::intersects(left, right)) {
// No intersection. Add smaller range.
if (left.start < right.start) {
- addRange(out, left.start, left.end);
+ addRange(out, left.start, left.end); // Input is sorted. Never returns false.
li += 2;
} else {
- addRange(out, right.start, right.end);
+ addRange(out, right.start, right.end); // Input is sorted. Never returns false.
ri += 2;
}
} else {
@@ -147,7 +157,7 @@
right = getRange(rRanges, ri);
}
}
- addRange(out, merged.start, merged.end);
+ addRange(out, merged.start, merged.end); // Input is sorted. Never returns false.
}
}
@@ -179,11 +189,15 @@
if (rangeOffset == 0) {
uint32_t delta = readU16(data, kHeaderSize + 2 * (2 * segCount + i));
if (((end + delta) & 0xffff) > end - start) {
- addRange(coverage, start, end + 1);
+ if (!addRange(coverage, start, end + 1)) {
+ return false;
+ }
} else {
for (uint32_t j = start; j < end + 1; j++) {
if (((j + delta) & 0xffff) != 0) {
- addRange(coverage, j, j + 1);
+ if (!addRange(coverage, j, j + 1)) {
+ return false;
+ }
}
}
}
@@ -197,7 +211,9 @@
}
uint32_t glyphId = readU16(data, actualRangeOffset);
if (glyphId != 0) {
- addRange(coverage, j, j + 1);
+ if (!addRange(coverage, j, j + 1)) {
+ return false;
+ }
}
}
}
@@ -238,10 +254,11 @@
}
if (end > MAX_UNICODE_CODE_POINT) {
// file is inclusive, vector is exclusive
- addRange(coverage, start, MAX_UNICODE_CODE_POINT + 1);
- return true;
+ return addRange(coverage, start, MAX_UNICODE_CODE_POINT + 1);
}
- addRange(coverage, start, end + 1); // file is inclusive, vector is exclusive
+ if (!addRange(coverage, start, end + 1)) { // file is inclusive, vector is exclusive
+ return false;
+ }
}
return true;
}
@@ -306,7 +323,9 @@
for (uint32_t i = 0; i < numRecords; ++i) {
const size_t recordOffset = kHeaderSize + kUVSMappingRecordSize * i;
const uint32_t codePoint = readU24(nonDefaultUVSTable, recordOffset);
- addRange(rangesFromNonDefaultUVSTable, codePoint, codePoint + 1);
+ if (!addRange(rangesFromNonDefaultUVSTable, codePoint, codePoint + 1)) {
+ return false;
+ }
}
}
@@ -342,7 +361,9 @@
// without variation selectors. We need to check the default glyph availability and
// exclude the codepoint if it is not supported by defualt cmap table.
if (baseCoverage.get(cp)) {
- addRange(rangesFromDefaultUVSTable, cp, cp + 1 /* exclusive */);
+ if (!addRange(rangesFromDefaultUVSTable, cp, cp + 1 /* exclusive */)) {
+ return false;
+ }
}
}
}
diff --git a/libs/minikin/LineBreaker.cpp b/libs/minikin/LineBreaker.cpp
index e75c7bf..d247644 100644
--- a/libs/minikin/LineBreaker.cpp
+++ b/libs/minikin/LineBreaker.cpp
@@ -400,7 +400,6 @@
size_t active = 0;
size_t nCand = mCandidates.size();
float width = mLineWidths.getLineWidth(0);
- float shortLineFactor = mJustified ? 0.75f : 0.5f;
float maxShrink = mJustified ? SHRINKABILITY * getSpaceWidth() : 0.0f;
// "i" iterates through candidates for the end of the line.
@@ -447,9 +446,6 @@
} else if (atEnd && mStrategy != kBreakStrategy_Balanced) {
// increase penalty for hyphen on last line
additionalPenalty = LAST_LINE_PENALTY_MULTIPLIER * mCandidates[j].penalty;
- // Penalize very short (< 1 - shortLineFactor of total width) lines.
- float underfill = delta - shortLineFactor * width;
- widthScore = underfill > 0 ? underfill * underfill : 0;
} else {
widthScore = delta * delta;
if (delta < 0) {
diff --git a/tests/perftests/Android.bp b/tests/perftests/Android.bp
index f8068e8..f8847ac 100644
--- a/tests/perftests/Android.bp
+++ b/tests/perftests/Android.bp
@@ -16,6 +16,7 @@
cc_benchmark {
name: "minikin_perftests",
+ test_suites: ["device-tests"],
cppflags: [
"-Werror",
"-Wall",
diff --git a/tests/perftests/AndroidTest.xml b/tests/perftests/AndroidTest.xml
new file mode 100644
index 0000000..dcbdcf1
--- /dev/null
+++ b/tests/perftests/AndroidTest.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+<configuration description="Config for minikin_perftests">
+ <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+ <option name="cleanup" value="true" />
+ <option name="push" value="minikin_perftests->/data/benchmarktest/minikin_perftests" />
+ </target_preparer>
+ <option name="test-suite-tag" value="apct" />
+ <test class="com.android.tradefed.testtype.GoogleBenchmarkTest" >
+ <option name="native-benchmark-device-path" value="/data/benchmarktest" />
+ <option name="benchmark-module-name" value="minikin_perftests" />
+ </test>
+</configuration>
diff --git a/tests/unittest/Android.bp b/tests/unittest/Android.bp
index b5f0248..40e268d 100644
--- a/tests/unittest/Android.bp
+++ b/tests/unittest/Android.bp
@@ -16,6 +16,7 @@
cc_test {
name: "minikin_tests",
+ test_suites: ["device-tests"],
data: [":minikin-test-data"],
header_libs: ["libminikin-headers-for-tests"],
diff --git a/tests/unittest/AndroidTest.xml b/tests/unittest/AndroidTest.xml
new file mode 100644
index 0000000..a073101
--- /dev/null
+++ b/tests/unittest/AndroidTest.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- 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.
+-->
+<configuration description="Config for minikin_tests">
+ <target_preparer class="com.android.tradefed.targetprep.PushFilePreparer">
+ <option name="cleanup" value="true" />
+ <option name="push" value="minikin_tests->/data/local/tmp/minikin_tests" />
+ </target_preparer>
+ <option name="test-suite-tag" value="apct" />
+ <test class="com.android.tradefed.testtype.GTest" >
+ <option name="native-test-device-path" value="/data/local/tmp" />
+ <option name="module-name" value="minikin_tests" />
+ </test>
+</configuration>
\ No newline at end of file
diff --git a/tests/unittest/CmapCoverageTest.cpp b/tests/unittest/CmapCoverageTest.cpp
index edf1ca1..fe2d7ba 100644
--- a/tests/unittest/CmapCoverageTest.cpp
+++ b/tests/unittest/CmapCoverageTest.cpp
@@ -314,6 +314,24 @@
EXPECT_EQ(0U, coverage.length());
EXPECT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat4Cmap(0, 0, std::vector<uint16_t>(
+ {'b', 'b', 'a', 'a'}));
+
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed range - partially readable");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat4Cmap(0, 0, std::vector<uint16_t>(
+ { 'a', 'a', 'c', 'c', 'b', 'b'}));
+
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
}
TEST(CmapCoverageTest, SingleFormat4) {
@@ -525,6 +543,22 @@
EXPECT_EQ(0U, coverage.length());
EXPECT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat12Cmap(
+ 0, 0, std::vector<uint32_t>({'b', 'b', 'a', 'a'}));
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed range - partially readable");
+ std::vector<uint8_t> cmap = CmapBuilder::buildSingleFormat12Cmap(
+ 0, 0, std::vector<uint32_t>({'a', 'a', 'c', 'c', 'b', 'b'}));
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ EXPECT_EQ(0U, coverage.length());
+ EXPECT_TRUE(vsTables.empty());
+ }
}
TEST(CmapCoverageTest, TableSelection_Priority) {
@@ -804,7 +838,7 @@
}
TEST(CmapCoverageTest, TableSelection_brokenVSTable) {
- std::vector<uint8_t> cmap12Table = buildCmapFormat12Table(std::vector<uint32_t>({'a', 'a'}));
+ std::vector<uint8_t> cmap12Table = buildCmapFormat12Table(std::vector<uint32_t>({'a', 'z'}));
{
SCOPED_TRACE("Too small cmap size");
std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
@@ -898,6 +932,62 @@
SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
ASSERT_TRUE(vsTables.empty());
}
+ {
+ SCOPED_TRACE("Reversed range in default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { 'b', 'b', 'a', 'a' }, { } }
+ }));
+ CmapBuilder builder(2);
+ builder.appendTable(3, 1, cmap12Table);
+ builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+ std::vector<uint8_t> cmap = builder.build();
+
+ std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ ASSERT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed range in default UVS table - partially readable");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { 'a', 'a', 'c', 'c', 'b', 'b' }, { } }
+ }));
+ CmapBuilder builder(2);
+ builder.appendTable(3, 1, cmap12Table);
+ builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+ std::vector<uint8_t> cmap = builder.build();
+
+ std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ ASSERT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed mapping entries in non default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { }, { 'b', 'a' } }
+ }));
+ CmapBuilder builder(2);
+ builder.appendTable(3, 1, cmap12Table);
+ builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+ std::vector<uint8_t> cmap = builder.build();
+
+ std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ ASSERT_TRUE(vsTables.empty());
+ }
+ {
+ SCOPED_TRACE("Reversed mapping entries in non default UVS table");
+ std::vector<uint8_t> vsTable = buildCmapFormat14Table(std::vector<VariationSelectorRecord>({
+ { 0xFE0F, { }, { 'a', 'c', 'b' } }
+ }));
+ CmapBuilder builder(2);
+ builder.appendTable(3, 1, cmap12Table);
+ builder.appendTable(VS_PLATFORM_ID, VS_ENCODING_ID, vsTable);
+ std::vector<uint8_t> cmap = builder.build();
+
+ std::vector<std::unique_ptr<SparseBitSet>> vsTables;
+ SparseBitSet coverage = CmapCoverage::getCoverage(cmap.data(), cmap.size(), &vsTables);
+ ASSERT_TRUE(vsTables.empty());
+ }
}
TEST(CmapCoverageTest, TableSelection_brokenVSTable_bestEffort) {
