From a0c3459c527f09d167b8d09bf6dd7010d5def2fa Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Tue, 16 Nov 2021 16:30:12 -0800 Subject: [PATCH 1/8] accessibility number formatting improvements --- .../accessibility/base/string_utils.cc | 50 +++++++++++++----- third_party/accessibility/base/string_utils.h | 16 ++++-- .../base/string_utils_unittest.cc | 51 ++++++++++++++++--- 3 files changed, 95 insertions(+), 22 deletions(-) diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 0dff8dd88a7ee..ba2caa87ea103 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -4,8 +4,11 @@ #include "string_utils.h" +#include #include +#include #include +#include #include #include #include @@ -48,8 +51,8 @@ std::wstring UTF16ToWide(const std::u16string& src) { return std::wstring(src.begin(), src.end()); } -std::u16string NumberToString16(float number) { - return ASCIIToUTF16(NumberToString(number)); +std::u16string NumberToString16(float number, int precision) { + return ASCIIToUTF16(NumberToString(number, precision)); } std::u16string NumberToString16(int32_t number) { @@ -60,8 +63,8 @@ std::u16string NumberToString16(unsigned int number) { return ASCIIToUTF16(NumberToString(number)); } -std::u16string NumberToString16(double number) { - return ASCIIToUTF16(NumberToString(number)); +std::u16string NumberToString16(double number, int precision) { + return ASCIIToUTF16(NumberToString(number, precision)); } std::string NumberToString(int32_t number) { @@ -72,16 +75,39 @@ std::string NumberToString(unsigned int number) { return std::to_string(number); } -std::string NumberToString(float number) { - // TODO(gw280): Format decimals to the shortest reasonable representation. - // See: https://github.com/flutter/flutter/issues/78460 - return std::to_string(number); +std::string NumberToString(float number, int precision) { + return NumberToString(static_cast(number), precision); } -std::string NumberToString(double number) { - // TODO(gw280): Format decimals to the shortest reasonable representation. - // See: https://github.com/flutter/flutter/issues/78460 - return std::to_string(number); +std::string NumberToString(double number, int precision) { + if (number == 0.0) { + return "0"; + } + + std::stringstream ss; + ss.precision(precision); + ss << std::fixed << number; + std::string str = ss.str(); + + int strip = 0; + // Strip any trailing zeros, including the decimal marker if there are nothing + // but zeros after the decimal. + for (std::string::reverse_iterator it = str.rbegin(); it != str.rend() - 1; + ++it) { + if (*it == '0') { + strip++; + } else if (*it == '.' || *it == ',') { + strip++; + // Don't keep stripping once we hit a decimal marker. + break; + } else { + break; + } + } + if (strip != 0) { + return str.substr(0, str.length() - strip); + } + return str; } std::string JoinString(std::vector tokens, std::string delimiter) { diff --git a/third_party/accessibility/base/string_utils.h b/third_party/accessibility/base/string_utils.h index ab1086c83881d..c1203c966f42f 100644 --- a/third_party/accessibility/base/string_utils.h +++ b/third_party/accessibility/base/string_utils.h @@ -13,6 +13,10 @@ namespace base { constexpr char16_t kWhitespaceUTF16 = u' '; +// The default number of digits of precision after the decimal to use when +// converting numbers to strings. +constexpr int kDefaultStringPrecision = 6; + // Return a C++ string given printf-like input. template std::string StringPrintf(const std::string& format, Args... args) { @@ -29,15 +33,19 @@ std::string UTF16ToUTF8(std::u16string src); std::u16string WideToUTF16(const std::wstring& src); std::wstring UTF16ToWide(const std::u16string& src); -std::u16string NumberToString16(float number); std::u16string NumberToString16(unsigned int number); std::u16string NumberToString16(int32_t number); -std::u16string NumberToString16(double number); +std::u16string NumberToString16(float number, + int precision = kDefaultStringPrecision); +std::u16string NumberToString16(double number, + int precision = kDefaultStringPrecision); std::string NumberToString(unsigned int number); std::string NumberToString(int32_t number); -std::string NumberToString(float number); -std::string NumberToString(double number); +std::string NumberToString(float number, + int precision = kDefaultStringPrecision); +std::string NumberToString(double number, + int precision = kDefaultStringPrecision); std::string ToUpperASCII(std::string str); std::string ToLowerASCII(std::string str); diff --git a/third_party/accessibility/base/string_utils_unittest.cc b/third_party/accessibility/base/string_utils_unittest.cc index dab09ead59833..eeb64425f5c8b 100644 --- a/third_party/accessibility/base/string_utils_unittest.cc +++ b/third_party/accessibility/base/string_utils_unittest.cc @@ -6,6 +6,7 @@ #include #include +#include #include "base/logging.h" #include "gtest/gtest.h" @@ -43,16 +44,54 @@ TEST(StringUtilsTest, canUTF16ToUTF8) { TEST(StringUtilsTest, canNumberToString16) { float number = 1.123; - EXPECT_EQ(NumberToString16(number).compare(u"1.123000"), 0); + EXPECT_EQ(NumberToString16(number).compare(u"1.123"), 0); } -TEST(StringUtilsTest, canNumberToString) { - float f = 1.123; - EXPECT_EQ(NumberToString(f).compare("1.123000"), 0); +TEST(StringUtilsTest, numberToStringSimplifiesOutput) { + double d0 = 0.0; + EXPECT_STREQ(NumberToString(d0).c_str(), "0"); + float f0 = 0.0f; + EXPECT_STREQ(NumberToString(f0).c_str(), "0"); + double d1 = 1.123; + EXPECT_STREQ(NumberToString(d1).c_str(), "1.123"); + float f1 = 1.123f; + EXPECT_STREQ(NumberToString(f1).c_str(), "1.123"); + double d2 = -1.123; + EXPECT_STREQ(NumberToString(d2).c_str(), "-1.123"); + float f2 = -1.123f; + EXPECT_STREQ(NumberToString(f2).c_str(), "-1.123"); + double d3 = 1.00001; + EXPECT_STREQ(NumberToString(d3).c_str(), "1.00001"); + float f3 = 1.00001f; + EXPECT_STREQ(NumberToString(f3).c_str(), "1.00001"); + double d4 = 1000.000001; + EXPECT_STREQ(NumberToString(d4).c_str(), "1000.000001"); + float f4 = 10.00001f; + EXPECT_STREQ(NumberToString(f4).c_str(), "10.00001"); + double d5 = 1.0 + 1e-8; + EXPECT_STREQ(NumberToString(d5).c_str(), "1"); + float f5 = 1.0f + 1e-8f; + EXPECT_STREQ(NumberToString(f5).c_str(), "1"); + double d6 = 1e-6; + EXPECT_STREQ(NumberToString(d6).c_str(), "0.000001"); + float f6 = 1e-6f; + EXPECT_STREQ(NumberToString(f6).c_str(), "0.000001"); + double d7 = 1e-8; + EXPECT_STREQ(NumberToString(d7).c_str(), "0"); + float f7 = 1e-8f; + EXPECT_STREQ(NumberToString(f7).c_str(), "0"); + double d8 = 100.0; + EXPECT_STREQ(NumberToString(d8).c_str(), "100"); + float f8 = 100.0f; + EXPECT_STREQ(NumberToString(f8).c_str(), "100"); + double d9 = 1.0 + 1e-7; + EXPECT_STREQ(NumberToString(d9, 7).c_str(), "1.0000001"); + float f9 = 1.0f + 1e-7f; + EXPECT_STREQ(NumberToString(f9, 7).c_str(), "1.0000001"); unsigned int s = 11; - EXPECT_EQ(NumberToString(s).compare("11"), 0); + EXPECT_STREQ(NumberToString(s).c_str(), "11"); int32_t i = -23; - EXPECT_EQ(NumberToString(i).compare("-23"), 0); + EXPECT_STREQ(NumberToString(i).c_str(), "-23"); } } // namespace base From d8509ab1b54aa524de54c9463e0a94456234c0bd Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 1 Dec 2021 16:53:17 -0800 Subject: [PATCH 2/8] Add double-conversion DEP --- DEPS | 3 +++ sky/tools/roll/roll.py | 1 + third_party/accessibility/base/BUILD.gn | 6 ++++++ third_party/accessibility/base/string_utils.cc | 2 ++ 4 files changed, 12 insertions(+) diff --git a/DEPS b/DEPS index aaf54cedb8c9b..e9a8786b3f761 100644 --- a/DEPS +++ b/DEPS @@ -369,6 +369,9 @@ deps = { 'src/third_party/freetype2': Var('flutter_git') + '/third_party/freetype2' + '@' + '1f03c1b2d7f2ae832a4fbe9d12bd96c3c15bbece', + 'src/third_party/double-conversion': + Var('flutter_git') + '/third_party/double-conversion' + '@' + '162356679e310566c8c36cb76703d9cfabe4cee8', + 'src/third_party/root_certificates': Var('dart_git') + '/root_certificates.git' + '@' + Var('dart_root_certificates_rev'), diff --git a/sky/tools/roll/roll.py b/sky/tools/roll/roll.py index a4d03aabdc2d2..bcab68d2dda5b 100755 --- a/sky/tools/roll/roll.py +++ b/sky/tools/roll/roll.py @@ -49,6 +49,7 @@ 'third_party/junit', 'third_party/libjpeg', 'third_party/libpng', + 'third_party/double-conversion', 'third_party/markupsafe', 'third_party/mesa', 'third_party/mockito', diff --git a/third_party/accessibility/base/BUILD.gn b/third_party/accessibility/base/BUILD.gn index c630ba7391b59..6f8aab91b3ce9 100644 --- a/third_party/accessibility/base/BUILD.gn +++ b/third_party/accessibility/base/BUILD.gn @@ -4,6 +4,10 @@ import("//flutter/common/config.gni") +config("double_conversion_config") { + include_dirs = [ "//third_party/double-conversion" ] +} + source_set("base") { visibility = [ "//flutter/third_party/accessibility/*" ] include_dirs = [ "//flutter/third_party/accessibility" ] @@ -53,5 +57,7 @@ source_set("base") { public_deps = [ "numerics", "//flutter/third_party/accessibility/ax_build", + "//third_party/double-conversion", + ":double_conversion_config", ] } diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index ba2caa87ea103..304336ec63394 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -13,6 +13,8 @@ #include #include +#include "double-conversion/double-conversion.h" + #if defined(_WIN32) #include "base/win/string_conversion.h" #endif From 524ee9c14f9fb698e0d2fdfad14f435cec98f776 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Wed, 1 Dec 2021 23:53:28 -0800 Subject: [PATCH 3/8] Use double-conversion from the dart runtime --- DEPS | 3 -- sky/packages/sky_engine/lib/_embedder.yaml | 41 ++++++++--------- third_party/accessibility/base/BUILD.gn | 7 +-- .../accessibility/base/string_utils.cc | 45 ++++++++++++++----- .../base/string_utils_unittest.cc | 4 +- 5 files changed, 59 insertions(+), 41 deletions(-) diff --git a/DEPS b/DEPS index e9a8786b3f761..aaf54cedb8c9b 100644 --- a/DEPS +++ b/DEPS @@ -369,9 +369,6 @@ deps = { 'src/third_party/freetype2': Var('flutter_git') + '/third_party/freetype2' + '@' + '1f03c1b2d7f2ae832a4fbe9d12bd96c3c15bbece', - 'src/third_party/double-conversion': - Var('flutter_git') + '/third_party/double-conversion' + '@' + '162356679e310566c8c36cb76703d9cfabe4cee8', - 'src/third_party/root_certificates': Var('dart_git') + '/root_certificates.git' + '@' + Var('dart_root_certificates_rev'), diff --git a/sky/packages/sky_engine/lib/_embedder.yaml b/sky/packages/sky_engine/lib/_embedder.yaml index c59d3623cd678..b10891fd4787f 100644 --- a/sky/packages/sky_engine/lib/_embedder.yaml +++ b/sky/packages/sky_engine/lib/_embedder.yaml @@ -1,26 +1,27 @@ -# This file is suitable for use within the tree. A different _embedder.yaml -# is generated by the BUILD.gn in this directory. Changes here must be -# mirrored there. +# This file is generated by //flutter/sky/packages/sky_engine:_embedder_yaml +# Do not modify this file directly. Instead, update the build file. + embedded_libs: - "dart:async": "../../../../../third_party/dart/sdk/lib/async/async.dart" - "dart:collection": "../../../../../third_party/dart/sdk/lib/collection/collection.dart" - "dart:convert": "../../../../../third_party/dart/sdk/lib/convert/convert.dart" - "dart:core": "../../../../../third_party/dart/sdk/lib/core/core.dart" - "dart:developer": "../../../../../third_party/dart/sdk/lib/developer/developer.dart" - "dart:ffi": "../../../../../third_party/dart/sdk/lib/ffi/ffi.dart" - "dart:html": "../../../../../third_party/dart/sdk/lib/html/html_dart2js.dart" - "dart:io": "../../../../../third_party/dart/sdk/lib/io/io.dart" - "dart:isolate": "../../../../../third_party/dart/sdk/lib/isolate/isolate.dart" - "dart:js": "../../../../../third_party/dart/sdk/lib/js/js.dart" - "dart:js_util": "../../../../../third_party/dart/sdk/lib/js_util/js_util.dart" - "dart:math": "../../../../../third_party/dart/sdk/lib/math/math.dart" - "dart:typed_data": "../../../../../third_party/dart/sdk/lib/typed_data/typed_data.dart" - "dart:ui": "../../../../lib/ui/ui.dart" + "dart:async": "async/async.dart" + "dart:collection": "collection/collection.dart" + "dart:convert": "convert/convert.dart" + "dart:core": "core/core.dart" + "dart:developer": "developer/developer.dart" + "dart:ffi": "ffi/ffi.dart" + "dart:html": "html/html_dart2js.dart" + "dart:io": "io/io.dart" + "dart:isolate": "isolate/isolate.dart" + "dart:js": "js/js.dart" + "dart:js_util": "js_util/js_util.dart" + "dart:math": "math/math.dart" + "dart:typed_data": "typed_data/typed_data.dart" + "dart:ui": "ui/ui.dart" - "dart:_http": "../../../../../third_party/dart/sdk/lib/_http/http.dart" - "dart:_interceptors": "../../../../../third_party/dart/sdk/lib/_interceptors/interceptors.dart" + "dart:_http": "_http/http.dart" + "dart:_interceptors": "_interceptors/interceptors.dart" # The _internal library is needed as some implementations bleed into the # public API, e.g. List being Iterable by virtue of implementing # EfficientLengthIterable. Not including this library yields analysis errors. - "dart:_internal": "../../../../../third_party/dart/sdk/lib/internal/internal.dart" + "dart:_internal": "internal/internal.dart" + "dart:_spirv": "spirv/spirv.dart" "dart:nativewrappers": "_empty.dart" diff --git a/third_party/accessibility/base/BUILD.gn b/third_party/accessibility/base/BUILD.gn index 6f8aab91b3ce9..0897fa07224b0 100644 --- a/third_party/accessibility/base/BUILD.gn +++ b/third_party/accessibility/base/BUILD.gn @@ -4,10 +4,6 @@ import("//flutter/common/config.gni") -config("double_conversion_config") { - include_dirs = [ "//third_party/double-conversion" ] -} - source_set("base") { visibility = [ "//flutter/third_party/accessibility/*" ] include_dirs = [ "//flutter/third_party/accessibility" ] @@ -57,7 +53,6 @@ source_set("base") { public_deps = [ "numerics", "//flutter/third_party/accessibility/ax_build", - "//third_party/double-conversion", - ":double_conversion_config", + "//third_party/dart/runtime/third_party/double-conversion/src:libdouble_conversion", ] } diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 304336ec63394..eb60009984c48 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -13,7 +13,7 @@ #include #include -#include "double-conversion/double-conversion.h" +#include "third_party/dart/runtime/third_party/double-conversion/src/double-conversion.h" #if defined(_WIN32) #include "base/win/string_conversion.h" @@ -23,6 +23,20 @@ namespace base { +namespace { +char const kExponentChar = 'e'; +const char* const kInfinitySymbol = "Infinity"; +const char* const kNaNSymbol = "NaN"; + +// The USE(x) template is used to silence C++ compiler warnings issued +// for unused variables. +template +static inline void USE(T&&) {} +} // namespace + +using double_conversion::DoubleToStringConverter; +using double_conversion::StringBuilder; + std::u16string ASCIIToUTF16(std::string src) { return std::u16string(src.begin(), src.end()); } @@ -85,17 +99,26 @@ std::string NumberToString(double number, int precision) { if (number == 0.0) { return "0"; } - - std::stringstream ss; - ss.precision(precision); - ss << std::fixed << number; - std::string str = ss.str(); - + static const int kMinPrecisionDigits = 0; + static const int kMaxPrecisionDigits = 21; + static const int kConversionFlags = DoubleToStringConverter::NO_FLAGS; + const int kBufferSize = 128; + + ASSERT(kMinPrecisionDigits <= precision && precision <= kMaxPrecisionDigits); + + const DoubleToStringConverter converter( + kConversionFlags, kInfinitySymbol, kNaNSymbol, kExponentChar, 0, 0, 0, 0); + std::vector char_buffer(kBufferSize); + char_buffer[kBufferSize - 1] = '\0'; + StringBuilder builder(char_buffer.data(), kBufferSize); + bool status = converter.ToFixed(number, precision, &builder); + ASSERT(status); + std::string result = std::string(builder.Finalize()); int strip = 0; // Strip any trailing zeros, including the decimal marker if there are nothing // but zeros after the decimal. - for (std::string::reverse_iterator it = str.rbegin(); it != str.rend() - 1; - ++it) { + for (std::string::reverse_iterator it = result.rbegin(); + it != result.rend() - 1; ++it) { if (*it == '0') { strip++; } else if (*it == '.' || *it == ',') { @@ -107,9 +130,9 @@ std::string NumberToString(double number, int precision) { } } if (strip != 0) { - return str.substr(0, str.length() - strip); + return result.substr(0, result.length() - strip); } - return str; + return result; } std::string JoinString(std::vector tokens, std::string delimiter) { diff --git a/third_party/accessibility/base/string_utils_unittest.cc b/third_party/accessibility/base/string_utils_unittest.cc index eeb64425f5c8b..3b8f132d3b265 100644 --- a/third_party/accessibility/base/string_utils_unittest.cc +++ b/third_party/accessibility/base/string_utils_unittest.cc @@ -44,7 +44,7 @@ TEST(StringUtilsTest, canUTF16ToUTF8) { TEST(StringUtilsTest, canNumberToString16) { float number = 1.123; - EXPECT_EQ(NumberToString16(number).compare(u"1.123"), 0); + EXPECT_EQ(NumberToString16(number), std::u16string(u"1.123")); } TEST(StringUtilsTest, numberToStringSimplifiesOutput) { @@ -88,6 +88,8 @@ TEST(StringUtilsTest, numberToStringSimplifiesOutput) { EXPECT_STREQ(NumberToString(d9, 7).c_str(), "1.0000001"); float f9 = 1.0f + 1e-7f; EXPECT_STREQ(NumberToString(f9, 7).c_str(), "1.0000001"); + EXPECT_STREQ(NumberToString(d9, 0).c_str(), "1"); + EXPECT_STREQ(NumberToString(f9, 0).c_str(), "1"); unsigned int s = 11; EXPECT_STREQ(NumberToString(s).c_str(), "11"); int32_t i = -23; From c391fc0783a952f9db397010fa22752a9106d217 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 2 Dec 2021 09:34:23 -0800 Subject: [PATCH 4/8] Use exponential notation sometimes, strip trailing zeros from mantissa too --- .../accessibility/base/string_utils.cc | 60 +++++++++++-------- .../base/string_utils_unittest.cc | 12 +++- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index eb60009984c48..699b649e5a869 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -28,10 +28,26 @@ char const kExponentChar = 'e'; const char* const kInfinitySymbol = "Infinity"; const char* const kNaNSymbol = "NaN"; -// The USE(x) template is used to silence C++ compiler warnings issued -// for unused variables. -template -static inline void USE(T&&) {} +// Strip any trailing zeros, including the decimal marker if there are nothing +// but zeros after the decimal. +std::string StripTrailingZeros(const std::string& str) { + int strip = 0; + for (auto it = str.rbegin(); it != str.rend() - 1; ++it) { + if (*it == '0') { + strip++; + } else if (*it == '.' || *it == ',') { + strip++; + // Don't keep stripping once we hit a decimal marker. + break; + } else { + break; + } + } + if (strip != 0) { + return str.substr(0, str.length() - strip); + } + return str; +} } // namespace using double_conversion::DoubleToStringConverter; @@ -101,6 +117,7 @@ std::string NumberToString(double number, int precision) { } static const int kMinPrecisionDigits = 0; static const int kMaxPrecisionDigits = 21; + static const double kMaxFixed = 10e13; static const int kConversionFlags = DoubleToStringConverter::NO_FLAGS; const int kBufferSize = 128; @@ -111,26 +128,21 @@ std::string NumberToString(double number, int precision) { std::vector char_buffer(kBufferSize); char_buffer[kBufferSize - 1] = '\0'; StringBuilder builder(char_buffer.data(), kBufferSize); - bool status = converter.ToFixed(number, precision, &builder); - ASSERT(status); - std::string result = std::string(builder.Finalize()); - int strip = 0; - // Strip any trailing zeros, including the decimal marker if there are nothing - // but zeros after the decimal. - for (std::string::reverse_iterator it = result.rbegin(); - it != result.rend() - 1; ++it) { - if (*it == '0') { - strip++; - } else if (*it == '.' || *it == ',') { - strip++; - // Don't keep stripping once we hit a decimal marker. - break; - } else { - break; - } - } - if (strip != 0) { - return result.substr(0, result.length() - strip); + std::string result; + double magnitude = fabs(number); + if (magnitude < pow(10, -precision) || magnitude > kMaxFixed) { + bool status = converter.ToExponential(number, precision, &builder); + ASSERT(status); + result = std::string(builder.Finalize()); + size_t exponent_index = result.rfind(kExponentChar); + // Strip trailing zeros from just the mantissa. + result = StripTrailingZeros(result.substr(0, exponent_index)) + + result.substr(exponent_index); + } else { + bool status = converter.ToFixed(number, precision, &builder); + ASSERT(status); + + result = StripTrailingZeros(std::string(builder.Finalize())); } return result; } diff --git a/third_party/accessibility/base/string_utils_unittest.cc b/third_party/accessibility/base/string_utils_unittest.cc index 3b8f132d3b265..7a07af5bdfb07 100644 --- a/third_party/accessibility/base/string_utils_unittest.cc +++ b/third_party/accessibility/base/string_utils_unittest.cc @@ -75,11 +75,13 @@ TEST(StringUtilsTest, numberToStringSimplifiesOutput) { double d6 = 1e-6; EXPECT_STREQ(NumberToString(d6).c_str(), "0.000001"); float f6 = 1e-6f; - EXPECT_STREQ(NumberToString(f6).c_str(), "0.000001"); + // This is different from the double version because of + // the precision difference in floats vs doubles. + EXPECT_STREQ(NumberToString(f6).c_str(), "1e-6"); double d7 = 1e-8; - EXPECT_STREQ(NumberToString(d7).c_str(), "0"); + EXPECT_STREQ(NumberToString(d7).c_str(), "1e-8"); float f7 = 1e-8f; - EXPECT_STREQ(NumberToString(f7).c_str(), "0"); + EXPECT_STREQ(NumberToString(f7).c_str(), "1e-8"); double d8 = 100.0; EXPECT_STREQ(NumberToString(d8).c_str(), "100"); float f8 = 100.0f; @@ -90,6 +92,10 @@ TEST(StringUtilsTest, numberToStringSimplifiesOutput) { EXPECT_STREQ(NumberToString(f9, 7).c_str(), "1.0000001"); EXPECT_STREQ(NumberToString(d9, 0).c_str(), "1"); EXPECT_STREQ(NumberToString(f9, 0).c_str(), "1"); + double d10 = 0.00000012345678; + EXPECT_STREQ(NumberToString(d10, 6).c_str(), "1.234568e-7"); + float f10 = 0.00000012345678f; + EXPECT_STREQ(NumberToString(f10, 6).c_str(), "1.234568e-7"); unsigned int s = 11; EXPECT_STREQ(NumberToString(s).c_str(), "11"); int32_t i = -23; From ec633f70811d716266e9bc74bb8242be5b1ceee4 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 2 Dec 2021 09:43:43 -0800 Subject: [PATCH 5/8] Roll back unnecessary changes --- sky/packages/sky_engine/lib/_embedder.yaml | 41 +++++++++---------- sky/tools/roll/roll.py | 1 - .../accessibility/base/string_utils.cc | 4 -- 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/sky/packages/sky_engine/lib/_embedder.yaml b/sky/packages/sky_engine/lib/_embedder.yaml index b10891fd4787f..c59d3623cd678 100644 --- a/sky/packages/sky_engine/lib/_embedder.yaml +++ b/sky/packages/sky_engine/lib/_embedder.yaml @@ -1,27 +1,26 @@ -# This file is generated by //flutter/sky/packages/sky_engine:_embedder_yaml -# Do not modify this file directly. Instead, update the build file. - +# This file is suitable for use within the tree. A different _embedder.yaml +# is generated by the BUILD.gn in this directory. Changes here must be +# mirrored there. embedded_libs: - "dart:async": "async/async.dart" - "dart:collection": "collection/collection.dart" - "dart:convert": "convert/convert.dart" - "dart:core": "core/core.dart" - "dart:developer": "developer/developer.dart" - "dart:ffi": "ffi/ffi.dart" - "dart:html": "html/html_dart2js.dart" - "dart:io": "io/io.dart" - "dart:isolate": "isolate/isolate.dart" - "dart:js": "js/js.dart" - "dart:js_util": "js_util/js_util.dart" - "dart:math": "math/math.dart" - "dart:typed_data": "typed_data/typed_data.dart" - "dart:ui": "ui/ui.dart" + "dart:async": "../../../../../third_party/dart/sdk/lib/async/async.dart" + "dart:collection": "../../../../../third_party/dart/sdk/lib/collection/collection.dart" + "dart:convert": "../../../../../third_party/dart/sdk/lib/convert/convert.dart" + "dart:core": "../../../../../third_party/dart/sdk/lib/core/core.dart" + "dart:developer": "../../../../../third_party/dart/sdk/lib/developer/developer.dart" + "dart:ffi": "../../../../../third_party/dart/sdk/lib/ffi/ffi.dart" + "dart:html": "../../../../../third_party/dart/sdk/lib/html/html_dart2js.dart" + "dart:io": "../../../../../third_party/dart/sdk/lib/io/io.dart" + "dart:isolate": "../../../../../third_party/dart/sdk/lib/isolate/isolate.dart" + "dart:js": "../../../../../third_party/dart/sdk/lib/js/js.dart" + "dart:js_util": "../../../../../third_party/dart/sdk/lib/js_util/js_util.dart" + "dart:math": "../../../../../third_party/dart/sdk/lib/math/math.dart" + "dart:typed_data": "../../../../../third_party/dart/sdk/lib/typed_data/typed_data.dart" + "dart:ui": "../../../../lib/ui/ui.dart" - "dart:_http": "_http/http.dart" - "dart:_interceptors": "_interceptors/interceptors.dart" + "dart:_http": "../../../../../third_party/dart/sdk/lib/_http/http.dart" + "dart:_interceptors": "../../../../../third_party/dart/sdk/lib/_interceptors/interceptors.dart" # The _internal library is needed as some implementations bleed into the # public API, e.g. List being Iterable by virtue of implementing # EfficientLengthIterable. Not including this library yields analysis errors. - "dart:_internal": "internal/internal.dart" - "dart:_spirv": "spirv/spirv.dart" + "dart:_internal": "../../../../../third_party/dart/sdk/lib/internal/internal.dart" "dart:nativewrappers": "_empty.dart" diff --git a/sky/tools/roll/roll.py b/sky/tools/roll/roll.py index bcab68d2dda5b..a4d03aabdc2d2 100755 --- a/sky/tools/roll/roll.py +++ b/sky/tools/roll/roll.py @@ -49,7 +49,6 @@ 'third_party/junit', 'third_party/libjpeg', 'third_party/libpng', - 'third_party/double-conversion', 'third_party/markupsafe', 'third_party/mesa', 'third_party/mockito', diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 699b649e5a869..34d76677bf768 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -4,11 +4,8 @@ #include "string_utils.h" -#include #include -#include #include -#include #include #include #include @@ -141,7 +138,6 @@ std::string NumberToString(double number, int precision) { } else { bool status = converter.ToFixed(number, precision, &builder); ASSERT(status); - result = StripTrailingZeros(std::string(builder.Finalize())); } return result; From e81ad07881cf3de029be8e4446fe8f161eb6e735 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Thu, 2 Dec 2021 12:34:46 -0800 Subject: [PATCH 6/8] Switch to using ToShortest in the double-conversion lib. --- .../accessibility/base/string_utils.cc | 107 +++++++----------- third_party/accessibility/base/string_utils.h | 16 +-- .../base/string_utils_unittest.cc | 83 +++++--------- 3 files changed, 77 insertions(+), 129 deletions(-) diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 34d76677bf768..9f0b562743bba 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -20,36 +20,48 @@ namespace base { +using double_conversion::DoubleToStringConverter; +using double_conversion::StringBuilder; + namespace { -char const kExponentChar = 'e'; -const char* const kInfinitySymbol = "Infinity"; -const char* const kNaNSymbol = "NaN"; - -// Strip any trailing zeros, including the decimal marker if there are nothing -// but zeros after the decimal. -std::string StripTrailingZeros(const std::string& str) { - int strip = 0; - for (auto it = str.rbegin(); it != str.rend() - 1; ++it) { - if (*it == '0') { - strip++; - } else if (*it == '.' || *it == ',') { - strip++; - // Don't keep stripping once we hit a decimal marker. - break; - } else { - break; - } +static char const kExponentChar = 'e'; +static const char* const kInfinitySymbol = "Infinity"; +static const char* const kNaNSymbol = "NaN"; + +// The number of digits after the decimal we allow before switching to +// exponential representation. +constexpr int kDecimalInShortestLow = -6; +// The number of digits before the decimal we allow before switching to +// exponential representation. +constexpr int kDecimalInShortestHigh = 12; +constexpr int kConversionFlags = + DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN; + +static const double_conversion::DoubleToStringConverter* +GetDoubleToStringConverter() { + static DoubleToStringConverter converter( + kConversionFlags, kInfinitySymbol, kNaNSymbol, kExponentChar, + kDecimalInShortestLow, kDecimalInShortestHigh, 0, 0); + return &converter; +} + +std::string NumberToStringImpl(double number, bool is_single_precision) { + if (number == 0.0) { + return "0"; } - if (strip != 0) { - return str.substr(0, str.length() - strip); + constexpr int kBufferSize = 128; + char char_buffer[kBufferSize]; + StringBuilder builder(char_buffer, sizeof(char_buffer)); + if (is_single_precision) { + GetDoubleToStringConverter()->ToShortestSingle(static_cast(number), + &builder); + } else { + GetDoubleToStringConverter()->ToShortest(number, &builder); } - return str; + return std::string(char_buffer, builder.position()); } } // namespace -using double_conversion::DoubleToStringConverter; -using double_conversion::StringBuilder; - std::u16string ASCIIToUTF16(std::string src) { return std::u16string(src.begin(), src.end()); } @@ -80,8 +92,8 @@ std::wstring UTF16ToWide(const std::u16string& src) { return std::wstring(src.begin(), src.end()); } -std::u16string NumberToString16(float number, int precision) { - return ASCIIToUTF16(NumberToString(number, precision)); +std::u16string NumberToString16(float number) { + return ASCIIToUTF16(NumberToString(number)); } std::u16string NumberToString16(int32_t number) { @@ -92,8 +104,8 @@ std::u16string NumberToString16(unsigned int number) { return ASCIIToUTF16(NumberToString(number)); } -std::u16string NumberToString16(double number, int precision) { - return ASCIIToUTF16(NumberToString(number, precision)); +std::u16string NumberToString16(double number) { + return ASCIIToUTF16(NumberToString(number)); } std::string NumberToString(int32_t number) { @@ -104,43 +116,12 @@ std::string NumberToString(unsigned int number) { return std::to_string(number); } -std::string NumberToString(float number, int precision) { - return NumberToString(static_cast(number), precision); +std::string NumberToString(float number) { + return NumberToStringImpl(number, true); } -std::string NumberToString(double number, int precision) { - if (number == 0.0) { - return "0"; - } - static const int kMinPrecisionDigits = 0; - static const int kMaxPrecisionDigits = 21; - static const double kMaxFixed = 10e13; - static const int kConversionFlags = DoubleToStringConverter::NO_FLAGS; - const int kBufferSize = 128; - - ASSERT(kMinPrecisionDigits <= precision && precision <= kMaxPrecisionDigits); - - const DoubleToStringConverter converter( - kConversionFlags, kInfinitySymbol, kNaNSymbol, kExponentChar, 0, 0, 0, 0); - std::vector char_buffer(kBufferSize); - char_buffer[kBufferSize - 1] = '\0'; - StringBuilder builder(char_buffer.data(), kBufferSize); - std::string result; - double magnitude = fabs(number); - if (magnitude < pow(10, -precision) || magnitude > kMaxFixed) { - bool status = converter.ToExponential(number, precision, &builder); - ASSERT(status); - result = std::string(builder.Finalize()); - size_t exponent_index = result.rfind(kExponentChar); - // Strip trailing zeros from just the mantissa. - result = StripTrailingZeros(result.substr(0, exponent_index)) + - result.substr(exponent_index); - } else { - bool status = converter.ToFixed(number, precision, &builder); - ASSERT(status); - result = StripTrailingZeros(std::string(builder.Finalize())); - } - return result; +std::string NumberToString(double number) { + return NumberToStringImpl(number, false); } std::string JoinString(std::vector tokens, std::string delimiter) { diff --git a/third_party/accessibility/base/string_utils.h b/third_party/accessibility/base/string_utils.h index c1203c966f42f..8ec5f05c245c8 100644 --- a/third_party/accessibility/base/string_utils.h +++ b/third_party/accessibility/base/string_utils.h @@ -13,10 +13,6 @@ namespace base { constexpr char16_t kWhitespaceUTF16 = u' '; -// The default number of digits of precision after the decimal to use when -// converting numbers to strings. -constexpr int kDefaultStringPrecision = 6; - // Return a C++ string given printf-like input. template std::string StringPrintf(const std::string& format, Args... args) { @@ -35,17 +31,13 @@ std::wstring UTF16ToWide(const std::u16string& src); std::u16string NumberToString16(unsigned int number); std::u16string NumberToString16(int32_t number); -std::u16string NumberToString16(float number, - int precision = kDefaultStringPrecision); -std::u16string NumberToString16(double number, - int precision = kDefaultStringPrecision); +std::u16string NumberToString16(float number); +std::u16string NumberToString16(double number); std::string NumberToString(unsigned int number); std::string NumberToString(int32_t number); -std::string NumberToString(float number, - int precision = kDefaultStringPrecision); -std::string NumberToString(double number, - int precision = kDefaultStringPrecision); +std::string NumberToString(float number); +std::string NumberToString(double number); std::string ToUpperASCII(std::string str); std::string ToLowerASCII(std::string str); diff --git a/third_party/accessibility/base/string_utils_unittest.cc b/third_party/accessibility/base/string_utils_unittest.cc index 7a07af5bdfb07..8f5d0fc3f6bb2 100644 --- a/third_party/accessibility/base/string_utils_unittest.cc +++ b/third_party/accessibility/base/string_utils_unittest.cc @@ -43,63 +43,38 @@ TEST(StringUtilsTest, canUTF16ToUTF8) { } TEST(StringUtilsTest, canNumberToString16) { - float number = 1.123; - EXPECT_EQ(NumberToString16(number), std::u16string(u"1.123")); + EXPECT_EQ(NumberToString16(1.123f), std::u16string(u"1.123")); } TEST(StringUtilsTest, numberToStringSimplifiesOutput) { - double d0 = 0.0; - EXPECT_STREQ(NumberToString(d0).c_str(), "0"); - float f0 = 0.0f; - EXPECT_STREQ(NumberToString(f0).c_str(), "0"); - double d1 = 1.123; - EXPECT_STREQ(NumberToString(d1).c_str(), "1.123"); - float f1 = 1.123f; - EXPECT_STREQ(NumberToString(f1).c_str(), "1.123"); - double d2 = -1.123; - EXPECT_STREQ(NumberToString(d2).c_str(), "-1.123"); - float f2 = -1.123f; - EXPECT_STREQ(NumberToString(f2).c_str(), "-1.123"); - double d3 = 1.00001; - EXPECT_STREQ(NumberToString(d3).c_str(), "1.00001"); - float f3 = 1.00001f; - EXPECT_STREQ(NumberToString(f3).c_str(), "1.00001"); - double d4 = 1000.000001; - EXPECT_STREQ(NumberToString(d4).c_str(), "1000.000001"); - float f4 = 10.00001f; - EXPECT_STREQ(NumberToString(f4).c_str(), "10.00001"); - double d5 = 1.0 + 1e-8; - EXPECT_STREQ(NumberToString(d5).c_str(), "1"); - float f5 = 1.0f + 1e-8f; - EXPECT_STREQ(NumberToString(f5).c_str(), "1"); - double d6 = 1e-6; - EXPECT_STREQ(NumberToString(d6).c_str(), "0.000001"); - float f6 = 1e-6f; - // This is different from the double version because of - // the precision difference in floats vs doubles. - EXPECT_STREQ(NumberToString(f6).c_str(), "1e-6"); - double d7 = 1e-8; - EXPECT_STREQ(NumberToString(d7).c_str(), "1e-8"); - float f7 = 1e-8f; - EXPECT_STREQ(NumberToString(f7).c_str(), "1e-8"); - double d8 = 100.0; - EXPECT_STREQ(NumberToString(d8).c_str(), "100"); - float f8 = 100.0f; - EXPECT_STREQ(NumberToString(f8).c_str(), "100"); - double d9 = 1.0 + 1e-7; - EXPECT_STREQ(NumberToString(d9, 7).c_str(), "1.0000001"); - float f9 = 1.0f + 1e-7f; - EXPECT_STREQ(NumberToString(f9, 7).c_str(), "1.0000001"); - EXPECT_STREQ(NumberToString(d9, 0).c_str(), "1"); - EXPECT_STREQ(NumberToString(f9, 0).c_str(), "1"); - double d10 = 0.00000012345678; - EXPECT_STREQ(NumberToString(d10, 6).c_str(), "1.234568e-7"); - float f10 = 0.00000012345678f; - EXPECT_STREQ(NumberToString(f10, 6).c_str(), "1.234568e-7"); - unsigned int s = 11; - EXPECT_STREQ(NumberToString(s).c_str(), "11"); - int32_t i = -23; - EXPECT_STREQ(NumberToString(i).c_str(), "-23"); + EXPECT_STREQ(NumberToString(0.0).c_str(), "0"); + EXPECT_STREQ(NumberToString(0.0f).c_str(), "0"); + EXPECT_STREQ(NumberToString(1.123).c_str(), "1.123"); + EXPECT_STREQ(NumberToString(1.123f).c_str(), "1.123"); + EXPECT_STREQ(NumberToString(-1.123).c_str(), "-1.123"); + EXPECT_STREQ(NumberToString(-1.123f).c_str(), "-1.123"); + EXPECT_STREQ(NumberToString(1.00001).c_str(), "1.00001"); + EXPECT_STREQ(NumberToString(1.00001f).c_str(), "1.00001"); + EXPECT_STREQ(NumberToString(1000.000001).c_str(), "1000.000001"); + EXPECT_STREQ(NumberToString(10.00001f).c_str(), "10.00001"); + EXPECT_STREQ(NumberToString(1.0 + 1e-8).c_str(), "1.00000001"); + EXPECT_STREQ(NumberToString(1.0f + 1e-8f).c_str(), "1"); + EXPECT_STREQ(NumberToString(1e-6).c_str(), "0.000001"); + EXPECT_STREQ(NumberToString(1e-6f).c_str(), "0.000001"); + EXPECT_STREQ(NumberToString(1e-8).c_str(), "1e-8"); + EXPECT_STREQ(NumberToString(1e-8f).c_str(), "1e-8"); + EXPECT_STREQ(NumberToString(100.0).c_str(), "100"); + EXPECT_STREQ(NumberToString(100.0f).c_str(), "100"); + EXPECT_STREQ(NumberToString(-1.0 - 1e-7).c_str(), "-1.0000001"); + EXPECT_STREQ(NumberToString(-1.0f - 1e-7f).c_str(), "-1.0000001"); + EXPECT_STREQ(NumberToString(0.00000012345678).c_str(), "1.2345678e-7"); + // Difference in output is due to differences in double and float precision. + EXPECT_STREQ(NumberToString(0.00000012345678f).c_str(), "1.2345679e-7"); + EXPECT_STREQ(NumberToString(-0.00000012345678).c_str(), "-1.2345678e-7"); + // Difference in output is due to differences in double and float precision. + EXPECT_STREQ(NumberToString(-0.00000012345678f).c_str(), "-1.2345679e-7"); + EXPECT_STREQ(NumberToString(static_cast(11)).c_str(), "11"); + EXPECT_STREQ(NumberToString(static_cast(-23)).c_str(), "-23"); } } // namespace base From 051605ef0da4dad8efffc7b3b122b5ae330c9540 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Fri, 3 Dec 2021 17:39:39 -0800 Subject: [PATCH 7/8] Clean up constness --- .../accessibility/base/string_utils.cc | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 9f0b562743bba..5f2bbdbc4c6f0 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -24,9 +24,9 @@ using double_conversion::DoubleToStringConverter; using double_conversion::StringBuilder; namespace { -static char const kExponentChar = 'e'; -static const char* const kInfinitySymbol = "Infinity"; -static const char* const kNaNSymbol = "NaN"; +constexpr char kExponentChar = 'e'; +constexpr char kInfinitySymbol[] = "Infinity"; +constexpr char kNaNSymbol[] = "NaN"; // The number of digits after the decimal we allow before switching to // exponential representation. @@ -37,28 +37,28 @@ constexpr int kDecimalInShortestHigh = 12; constexpr int kConversionFlags = DoubleToStringConverter::EMIT_POSITIVE_EXPONENT_SIGN; -static const double_conversion::DoubleToStringConverter* -GetDoubleToStringConverter() { +const DoubleToStringConverter& GetDoubleToStringConverter() { static DoubleToStringConverter converter( kConversionFlags, kInfinitySymbol, kNaNSymbol, kExponentChar, kDecimalInShortestLow, kDecimalInShortestHigh, 0, 0); - return &converter; + return converter; } std::string NumberToStringImpl(double number, bool is_single_precision) { if (number == 0.0) { return "0"; } + constexpr int kBufferSize = 128; - char char_buffer[kBufferSize]; - StringBuilder builder(char_buffer, sizeof(char_buffer)); + std::vector char_buffer(kBufferSize); + StringBuilder builder(char_buffer.data(), char_buffer.size()); if (is_single_precision) { - GetDoubleToStringConverter()->ToShortestSingle(static_cast(number), - &builder); + GetDoubleToStringConverter().ToShortestSingle(static_cast(number), + &builder); } else { - GetDoubleToStringConverter()->ToShortest(number, &builder); + GetDoubleToStringConverter().ToShortest(number, &builder); } - return std::string(char_buffer, builder.position()); + return std::string(char_buffer.data(), builder.position()); } } // namespace From 8e2600d6d6670f98c7d4a543be6f4274dcb5b342 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Mon, 6 Dec 2021 11:23:15 -0800 Subject: [PATCH 8/8] Switch to array instead of vector --- third_party/accessibility/base/string_utils.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/third_party/accessibility/base/string_utils.cc b/third_party/accessibility/base/string_utils.cc index 5f2bbdbc4c6f0..0152bbcbef65e 100644 --- a/third_party/accessibility/base/string_utils.cc +++ b/third_party/accessibility/base/string_utils.cc @@ -4,6 +4,7 @@ #include "string_utils.h" +#include #include #include #include @@ -50,7 +51,7 @@ std::string NumberToStringImpl(double number, bool is_single_precision) { } constexpr int kBufferSize = 128; - std::vector char_buffer(kBufferSize); + std::array char_buffer; StringBuilder builder(char_buffer.data(), char_buffer.size()); if (is_single_precision) { GetDoubleToStringConverter().ToShortestSingle(static_cast(number),