diff options
Diffstat (limited to 'chromium/ui/gfx')
72 files changed, 1666 insertions, 966 deletions
diff --git a/chromium/ui/gfx/BUILD.gn b/chromium/ui/gfx/BUILD.gn index e579efd759a..141803adf75 100644 --- a/chromium/ui/gfx/BUILD.gn +++ b/chromium/ui/gfx/BUILD.gn @@ -44,8 +44,6 @@ component("geometry_skia") { component("gfx") { sources = [ - "android/gfx_jni_registrar.cc", - "android/gfx_jni_registrar.h", "android/java_bitmap.cc", "android/java_bitmap.h", "android/view_configuration.cc", @@ -94,6 +92,7 @@ component("gfx") { "image/image_family.h", "image/image_ios.mm", "image/image_mac.mm", + "image/image_platform.h", "image/image_png_rep.cc", "image/image_png_rep.h", "image/image_skia.cc", @@ -206,6 +205,7 @@ component("gfx") { "canvas_skia_paint.h", "image/canvas_image_source.cc", "image/canvas_image_source.h", + "image/image_generic.cc", "image/image_skia_operations.cc", "image/image_skia_operations.h", "paint_throbber.cc", @@ -256,7 +256,7 @@ component("gfx") { ] # Text rendering conditions (complicated so separated out). - if (use_aura || is_mac || (is_android && enable_vr)) { + if (use_aura || is_mac || (is_android && enable_vr) || is_fuchsia) { # Mac doesn't use RenderTextHarfBuzz by default yet. sources += [ "harfbuzz_font_skia.cc", @@ -330,8 +330,7 @@ component("gfx") { sources -= [ "canvas_notimplemented.cc" ] } - # Desktop and Android+VR only. - if (use_aura || (!is_ios && !is_android) || (is_android && enable_vr)) { + if (!is_ios) { sources += [ "paint_vector_icon.cc", "paint_vector_icon.h", @@ -388,6 +387,14 @@ component("gfx") { if (use_cairo) { configs += [ "//build/config/linux/pangocairo" ] } + + if (is_fuchsia) { + sources += [ + "font_fallback_fuchsia.cc", + "font_render_params_fuchsia.cc", + "platform_font_fuchsia.cc", + ] + } } component("color_space") { @@ -483,6 +490,13 @@ source_set("selection_bound_sources") { ] } +# Depend on this to use buffer_types.h without pulling in all of gfx. +source_set("buffer_types") { + sources = [ + "buffer_types.h", + ] +} + # The GPU memory buffer stuff is separate from "gfx" to allow GPU-related # things to use these files without pulling in all of gfx, which includes large # things like Skia. @@ -510,7 +524,6 @@ source_set("memory_buffer_sources") { sources = [ "buffer_format_util.cc", "buffer_format_util.h", - "buffer_types.h", "client_native_pixmap.h", "client_native_pixmap_factory.cc", "client_native_pixmap_factory.h", @@ -529,13 +542,15 @@ source_set("memory_buffer_sources") { sources += [ "gpu_memory_buffer.cc", "gpu_memory_buffer.h", - "gpu_memory_buffer_tracing.cc", - "gpu_memory_buffer_tracing.h", ] } defines = [ "GFX_IMPLEMENTATION" ] + public_deps = [ + ":buffer_types", + ] + deps = [ ":gfx_switches", ":native_widget_types", @@ -702,11 +717,12 @@ test("gfx_unittests") { ] } + if (!is_ios) { + sources += [ "paint_vector_icon_unittest.cc" ] + } + if (!is_android && !is_ios) { - sources += [ - "paint_vector_icon_unittest.cc", - "render_text_unittest.cc", - ] + sources += [ "render_text_unittest.cc" ] } # TODO(jschuh): crbug.com/167187 fix size_t to int truncations. diff --git a/chromium/ui/gfx/DEPS b/chromium/ui/gfx/DEPS index a6891ad405f..808d12b3ae0 100644 --- a/chromium/ui/gfx/DEPS +++ b/chromium/ui/gfx/DEPS @@ -1,5 +1,6 @@ include_rules = [ "+base", + "+cc/base", "+cc/paint", "+device/vr/features/features.h", "+skia/ext", diff --git a/chromium/ui/gfx/android/view_configuration.cc b/chromium/ui/gfx/android/view_configuration.cc index a79f0f92aa3..11fe458ccab 100644 --- a/chromium/ui/gfx/android/view_configuration.cc +++ b/chromium/ui/gfx/android/view_configuration.cc @@ -175,8 +175,4 @@ int ViewConfiguration::GetMinScalingSpanInDips() { return g_view_configuration.Get().min_scaling_span_in_dips(); } -bool ViewConfiguration::RegisterViewConfiguration(JNIEnv* env) { - return RegisterNativesImpl(env); -} - } // namespace gfx diff --git a/chromium/ui/gfx/android/view_configuration.h b/chromium/ui/gfx/android/view_configuration.h index 4495a5eccc6..e7353de3d07 100644 --- a/chromium/ui/gfx/android/view_configuration.h +++ b/chromium/ui/gfx/android/view_configuration.h @@ -26,9 +26,6 @@ class GFX_EXPORT ViewConfiguration { static int GetDoubleTapSlopInDips(); static int GetMinScalingSpanInDips(); - - // Registers methods with JNI and returns true if succeeded. - static bool RegisterViewConfiguration(JNIEnv* env); }; } // namespace gfx diff --git a/chromium/ui/gfx/animation/animation_container.cc b/chromium/ui/gfx/animation/animation_container.cc index e06803d06e1..ef15c8f64c2 100644 --- a/chromium/ui/gfx/animation/animation_container.cc +++ b/chromium/ui/gfx/animation/animation_container.cc @@ -13,8 +13,9 @@ using base::TimeTicks; namespace gfx { AnimationContainer::AnimationContainer() - : last_tick_time_(base::TimeTicks::Now()), observer_(NULL) { -} + : last_tick_time_(base::TimeTicks::Now()), + min_timer_interval_count_(0), + observer_(NULL) {} AnimationContainer::~AnimationContainer() { // The animations own us and stop themselves before being deleted. If @@ -29,8 +30,12 @@ void AnimationContainer::Start(AnimationContainerElement* element) { if (elements_.empty()) { last_tick_time_ = base::TimeTicks::Now(); SetMinTimerInterval(element->GetTimerInterval()); + min_timer_interval_count_ = 1; } else if (element->GetTimerInterval() < min_timer_interval_) { SetMinTimerInterval(element->GetTimerInterval()); + min_timer_interval_count_ = 1; + } else if (element->GetTimerInterval() == min_timer_interval_) { + min_timer_interval_count_++; } element->SetStartTime(last_tick_time_); @@ -40,16 +45,27 @@ void AnimationContainer::Start(AnimationContainerElement* element) { void AnimationContainer::Stop(AnimationContainerElement* element) { DCHECK(elements_.count(element) > 0); // The element must be running. + base::TimeDelta interval = element->GetTimerInterval(); elements_.erase(element); if (elements_.empty()) { timer_.Stop(); + min_timer_interval_count_ = 0; if (observer_) observer_->AnimationContainerEmpty(this); - } else { - TimeDelta min_timer_interval = GetMinInterval(); - if (min_timer_interval > min_timer_interval_) - SetMinTimerInterval(min_timer_interval); + } else if (interval == min_timer_interval_) { + min_timer_interval_count_--; + + // If the last element at the current (minimum) timer interval has been + // removed then go find the new minimum and the number of elements at that + // same minimum. + if (min_timer_interval_count_ == 0) { + std::pair<base::TimeDelta, size_t> interval_count = + GetMinIntervalAndCount(); + DCHECK(interval_count.first > min_timer_interval_); + SetMinTimerInterval(interval_count.first); + min_timer_interval_count_ = interval_count.second; + } } } @@ -87,17 +103,30 @@ void AnimationContainer::SetMinTimerInterval(base::TimeDelta delta) { timer_.Start(FROM_HERE, min_timer_interval_, this, &AnimationContainer::Run); } -TimeDelta AnimationContainer::GetMinInterval() { +std::pair<TimeDelta, size_t> AnimationContainer::GetMinIntervalAndCount() + const { DCHECK(!elements_.empty()); + // Find the minimum interval and the number of elements sharing that same + // interval. It is tempting to create a map of intervals -> counts in order to + // make this O(log n) instead of O(n). However, profiling shows that this + // offers no practical performance gain (the most common case is that all + // elements in the set share the same interval). TimeDelta min; + size_t count = 1; Elements::const_iterator i = elements_.begin(); min = (*i)->GetTimerInterval(); for (++i; i != elements_.end(); ++i) { - if ((*i)->GetTimerInterval() < min) - min = (*i)->GetTimerInterval(); + auto interval = (*i)->GetTimerInterval(); + if (interval < min) { + min = interval; + count = 1; + } else if (interval == min) { + count++; + } } - return min; + + return std::make_pair(min, count); } } // namespace gfx diff --git a/chromium/ui/gfx/animation/animation_container.h b/chromium/ui/gfx/animation/animation_container.h index a19c0539d2f..58ad1ea1d01 100644 --- a/chromium/ui/gfx/animation/animation_container.h +++ b/chromium/ui/gfx/animation/animation_container.h @@ -5,8 +5,9 @@ #ifndef UI_GFX_ANIMATION_ANIMATION_CONTAINER_H_ #define UI_GFX_ANIMATION_ANIMATION_CONTAINER_H_ -#include <set> +#include <utility> +#include "base/containers/flat_set.h" #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/time/time.h" @@ -55,7 +56,13 @@ class ANIMATION_EXPORT AnimationContainer private: friend class base::RefCounted<AnimationContainer>; - typedef std::set<AnimationContainerElement*> Elements; + // This set is usually quite small so a flat_set is the most obvious choice. + // However, in extreme cases this can grow to 100s or even 1000s of elements. + // Since this set is duplicated on every call to 'Run' and indexed very + // frequently the cache locality of the vector is more important than the + // costlier (but rarer) insertion. Profiling shows that flat_set continues to + // perform best in these cases (up to 12x faster than std::set). + typedef base::flat_set<AnimationContainerElement*> Elements; ~AnimationContainer(); @@ -65,8 +72,9 @@ class ANIMATION_EXPORT AnimationContainer // Sets min_timer_interval_ and restarts the timer. void SetMinTimerInterval(base::TimeDelta delta); - // Returns the min timer interval of all the timers. - base::TimeDelta GetMinInterval(); + // Returns the min timer interval of all the timers, and the count of timers + // at that interval. + std::pair<base::TimeDelta, size_t> GetMinIntervalAndCount() const; // Represents one of two possible values: // . If only a single animation has been started and the timer hasn't yet @@ -77,8 +85,14 @@ class ANIMATION_EXPORT AnimationContainer // Set of elements (animations) being managed. Elements elements_; - // Minimum interval the timers run at. + // Minimum interval the timers run at, plus the number of timers that have + // been seen at that interval. The most common case is for all of the + // animations to run at 60Hz, in which case all of the intervals are the same. + // This acts as a cache of size 1, and when an animation stops and is removed + // it means that the linear scan for the new minimum timer can almost always + // be avoided. base::TimeDelta min_timer_interval_; + size_t min_timer_interval_count_; base::RepeatingTimer timer_; diff --git a/chromium/ui/gfx/animation/animation_unittest.cc b/chromium/ui/gfx/animation/animation_unittest.cc index 66e9d98e085..bf1cf0138f1 100644 --- a/chromium/ui/gfx/animation/animation_unittest.cc +++ b/chromium/ui/gfx/animation/animation_unittest.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/message_loop/message_loop.h" #include "base/run_loop.h" #include "base/test/scoped_task_environment.h" #include "build/build_config.h" @@ -79,7 +78,7 @@ class DeletingAnimationDelegate : public AnimationDelegate { public: void AnimationEnded(const Animation* animation) override { delete animation; - base::MessageLoop::current()->QuitWhenIdle(); + base::RunLoop::QuitCurrentWhenIdleDeprecated(); } }; diff --git a/chromium/ui/gfx/animation/linear_animation.cc b/chromium/ui/gfx/animation/linear_animation.cc index 2886a87aad5..7181214d0fe 100644 --- a/chromium/ui/gfx/animation/linear_animation.cc +++ b/chromium/ui/gfx/animation/linear_animation.cc @@ -21,6 +21,8 @@ static base::TimeDelta CalculateInterval(int frame_rate) { return base::TimeDelta::FromMicroseconds(timer_interval); } +const int LinearAnimation::kDefaultFrameRate = 60; + LinearAnimation::LinearAnimation(AnimationDelegate* delegate, int frame_rate) : LinearAnimation({}, frame_rate, delegate) {} diff --git a/chromium/ui/gfx/animation/linear_animation.h b/chromium/ui/gfx/animation/linear_animation.h index 9f3654d442a..d1393da0fd0 100644 --- a/chromium/ui/gfx/animation/linear_animation.h +++ b/chromium/ui/gfx/animation/linear_animation.h @@ -18,7 +18,7 @@ class AnimationDelegate; class ANIMATION_EXPORT LinearAnimation : public Animation { public: // Default frame rate (hz). - static const int kDefaultFrameRate = 60; + static const int kDefaultFrameRate; // Initializes everything except the duration. // diff --git a/chromium/ui/gfx/animation/test_animation_delegate.h b/chromium/ui/gfx/animation/test_animation_delegate.h index 6cc65afbc7e..3c40a8993e3 100644 --- a/chromium/ui/gfx/animation/test_animation_delegate.h +++ b/chromium/ui/gfx/animation/test_animation_delegate.h @@ -6,7 +6,7 @@ #define UI_GFX_ANIMATION_TEST_ANIMATION_DELEGATE_H_ #include "base/macros.h" -#include "base/message_loop/message_loop.h" +#include "base/run_loop.h" #include "ui/gfx/animation/animation_delegate.h" namespace gfx { @@ -20,13 +20,13 @@ class TestAnimationDelegate : public AnimationDelegate { virtual void AnimationEnded(const Animation* animation) { finished_ = true; - base::MessageLoop::current()->QuitWhenIdle(); + base::RunLoop::QuitCurrentWhenIdleDeprecated(); } virtual void AnimationCanceled(const Animation* animation) { finished_ = true; canceled_ = true; - base::MessageLoop::current()->QuitWhenIdle(); + base::RunLoop::QuitCurrentWhenIdleDeprecated(); } bool finished() const { diff --git a/chromium/ui/gfx/animation/tween.cc b/chromium/ui/gfx/animation/tween.cc index 1cebe30c4ae..1fced2c368a 100644 --- a/chromium/ui/gfx/animation/tween.cc +++ b/chromium/ui/gfx/animation/tween.cc @@ -98,8 +98,8 @@ uint8_t BlendColorComponents(uint8_t start, } double TimeDeltaDivide(base::TimeDelta dividend, base::TimeDelta divisor) { - return static_cast<double>(dividend.ToInternalValue()) / - static_cast<double>(divisor.ToInternalValue()); + return static_cast<double>(dividend.InMicroseconds()) / + static_cast<double>(divisor.InMicroseconds()); } } // namespace diff --git a/chromium/ui/gfx/buffer_types.h b/chromium/ui/gfx/buffer_types.h index 9cb5b7b6ee5..b35f587576f 100644 --- a/chromium/ui/gfx/buffer_types.h +++ b/chromium/ui/gfx/buffer_types.h @@ -37,12 +37,18 @@ enum class BufferFormat { // by the CPU. *_CPU_READ_WRITE_PERSISTENT adds the additional condition that // successive Map() calls (with Unmap() calls between) will return a pointer to // the same memory contents. SCANOUT implies GPU_READ_WRITE. +// *_VDA_WRITE is for cases where a video decode accellerator writes into +// the buffers. + // TODO(reveman): Add GPU_READ_WRITE for use-cases where SCANOUT is not // required. enum class BufferUsage { GPU_READ, SCANOUT, + // SCANOUT_CAMERA_READ_WRITE implies CPU_READ_WRITE. + SCANOUT_CAMERA_READ_WRITE, SCANOUT_CPU_READ_WRITE, + SCANOUT_VDA_WRITE, GPU_READ_CPU_READ_WRITE, // TODO(reveman): Merge this with GPU_READ_CPU_READ_WRITE when SurfaceTexture // backed buffers are single buffered and support it. diff --git a/chromium/ui/gfx/canvas.cc b/chromium/ui/gfx/canvas.cc index 414c2c90e8d..ff5f8a8373c 100644 --- a/chromium/ui/gfx/canvas.cc +++ b/chromium/ui/gfx/canvas.cc @@ -287,15 +287,19 @@ void Canvas::Draw1pxLine(PointF p1, PointF p2, SkColor color) { void Canvas::DrawCircle(const Point& center_point, int radius, const cc::PaintFlags& flags) { - DrawCircle(PointF(center_point), radius, flags); + canvas_->drawOval( + SkRect::MakeLTRB(center_point.x() - radius, center_point.y() - radius, + center_point.x() + radius, center_point.y() + radius), + flags); } void Canvas::DrawCircle(const PointF& center_point, float radius, const cc::PaintFlags& flags) { - canvas_->drawCircle(SkFloatToScalar(center_point.x()), - SkFloatToScalar(center_point.y()), - SkFloatToScalar(radius), flags); + canvas_->drawOval( + SkRect::MakeLTRB(center_point.x() - radius, center_point.y() - radius, + center_point.x() + radius, center_point.y() + radius), + flags); } void Canvas::DrawRoundRect(const Rect& rect, diff --git a/chromium/ui/gfx/color_space.cc b/chromium/ui/gfx/color_space.cc index 96b9306e739..f6352710c37 100644 --- a/chromium/ui/gfx/color_space.cc +++ b/chromium/ui/gfx/color_space.cc @@ -35,6 +35,14 @@ ColorSpace::ColorSpace(PrimaryID primaries, matrix_(matrix), range_(range) {} +ColorSpace::ColorSpace(PrimaryID primaries, + const SkColorSpaceTransferFn& fn, + MatrixID matrix, + RangeID range) + : primaries_(primaries), matrix_(matrix), range_(range) { + SetCustomTransferFunction(fn); +} + ColorSpace::ColorSpace(const ColorSpace& other) : primaries_(other.primaries_), transfer_(other.transfer_), @@ -92,17 +100,22 @@ ColorSpace ColorSpace::CreateCustom(const SkMatrix44& to_XYZD50, result.custom_primary_matrix_[3 * row + col] = to_XYZD50.get(row, col); } } - result.custom_transfer_params_[0] = fn.fA; - result.custom_transfer_params_[1] = fn.fB; - result.custom_transfer_params_[2] = fn.fC; - result.custom_transfer_params_[3] = fn.fD; - result.custom_transfer_params_[4] = fn.fE; - result.custom_transfer_params_[5] = fn.fF; - result.custom_transfer_params_[6] = fn.fG; - // TODO(ccameron): Use enums for near matches to know color spaces. + result.SetCustomTransferFunction(fn); return result; } +void ColorSpace::SetCustomTransferFunction(const SkColorSpaceTransferFn& fn) { + custom_transfer_params_[0] = fn.fA; + custom_transfer_params_[1] = fn.fB; + custom_transfer_params_[2] = fn.fC; + custom_transfer_params_[3] = fn.fD; + custom_transfer_params_[4] = fn.fE; + custom_transfer_params_[5] = fn.fF; + custom_transfer_params_[6] = fn.fG; + // TODO(ccameron): Use enums for near matches to know color spaces. + transfer_ = TransferID::CUSTOM; +} + // static ColorSpace ColorSpace::CreateCustom(const SkMatrix44& to_XYZD50, ColorSpace::TransferID transfer_id) { @@ -154,14 +167,23 @@ bool ColorSpace::operator==(const ColorSpace& other) const { if (primaries_ != other.primaries_ || transfer_ != other.transfer_ || matrix_ != other.matrix_ || range_ != other.range_) return false; - if (primaries_ == PrimaryID::CUSTOM && - memcmp(custom_primary_matrix_, other.custom_primary_matrix_, - sizeof(custom_primary_matrix_))) - return false; - if (transfer_ == TransferID::CUSTOM && - memcmp(custom_transfer_params_, other.custom_transfer_params_, - sizeof(custom_transfer_params_))) - return false; + if (primaries_ == PrimaryID::CUSTOM) { + if (memcmp(custom_primary_matrix_, other.custom_primary_matrix_, + sizeof(custom_primary_matrix_))) { + return false; + } + } + if (transfer_ == TransferID::CUSTOM) { + if (memcmp(custom_transfer_params_, other.custom_transfer_params_, + sizeof(custom_transfer_params_))) { + return false; + } + } + if (primaries_ == PrimaryID::ICC_BASED || + transfer_ == TransferID::ICC_BASED) { + if (icc_profile_id_ != other.icc_profile_id_) + return false; + } return true; } @@ -191,8 +213,12 @@ ColorSpace ColorSpace::GetParametricApproximation() const { // Query the ICC profile, if available, for the parametric approximation. ICCProfile icc_profile; - if (GetICCProfile(&icc_profile)) + if (icc_profile_id_ && GetICCProfile(&icc_profile)) { return icc_profile.GetParametricColorSpace(); + } else { + DLOG(ERROR) + << "Unable to acquire ICC profile for parametric approximation."; + } // Fall back to sRGB if the ICC profile is no longer cached. return CreateSRGB(); @@ -281,8 +307,10 @@ std::string ColorSpace::ToString() const { ss << ", transfer:"; if (transfer_ == TransferID::CUSTOM) { ss << "["; - for (size_t i = 0; i < 7; ++i) + for (size_t i = 0; i < 7; ++i) { ss << custom_transfer_params_[i]; + ss << ","; + } ss << "]"; } else { ss << static_cast<int>(transfer_); @@ -303,6 +331,26 @@ ColorSpace ColorSpace::GetAsFullRangeRGB() const { return result; } +ColorSpace ColorSpace::GetRasterColorSpace() const { + // Rasterization can only be done into parametric color spaces. + if (!IsParametric()) + return GetParametricApproximation(); + // Rasterization doesn't support more than 8 bit unorm values. If the output + // space has an extended range, use Display P3 for the rasterization space, + // to get a somewhat wider color gamut. + if (HasExtendedSkTransferFn()) + return CreateDisplayP3D65(); + return *this; +} + +ColorSpace ColorSpace::GetBlendingColorSpace() const { + // HDR output on windows requires output have a linear transfer function. + // Linear blending breaks the web, so use extended-sRGB for blending. + if (transfer_ == TransferID::LINEAR_HDR) + return CreateExtendedSRGB(); + return *this; +} + sk_sp<SkColorSpace> ColorSpace::ToSkColorSpace() const { // If we got a specific SkColorSpace from the ICCProfile that this color space // was created from, use that. @@ -423,6 +471,47 @@ bool ColorSpace::GetICCProfile(ICCProfile* icc_profile) const { return true; } +bool ColorSpace::GetICCProfileData(std::vector<char>* output_data) const { + if (!IsValid()) { + DLOG(ERROR) << "Cannot fetch ICCProfile for invalid space."; + return false; + } + if (matrix_ != MatrixID::RGB) { + DLOG(ERROR) << "Not creating non-RGB ICCProfile"; + return false; + } + if (range_ != RangeID::FULL) { + DLOG(ERROR) << "Not creating non-full-range ICCProfile"; + return false; + } + + // If this was created from an ICC profile, retrieve that exact profile. + ICCProfile icc_profile; + if (ICCProfile::FromId(icc_profile_id_, &icc_profile)) { + *output_data = icc_profile.data_; + return true; + } + + // Otherwise, construct an ICC profile based on the best approximated + // primaries and matrix. + SkMatrix44 to_XYZD50_matrix; + GetPrimaryMatrix(&to_XYZD50_matrix); + SkColorSpaceTransferFn fn; + if (!GetTransferFunction(&fn)) { + DLOG(ERROR) << "Failed to get ColorSpace transfer function for ICCProfile."; + return false; + } + sk_sp<SkData> data = SkICC::WriteToICC(fn, to_XYZD50_matrix); + if (!data || !data->size()) { + DLOG(ERROR) << "Failed to create SkICC."; + return false; + } + const char* data_as_char = reinterpret_cast<const char*>(data->data()); + output_data->insert(output_data->begin(), data_as_char, + data_as_char + data->size()); + return true; +} + void ColorSpace::GetPrimaryMatrix(SkMatrix44* to_XYZD50) const { SkColorSpacePrimaries primaries = {0}; switch (primaries_) { @@ -621,16 +710,6 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const { case ColorSpace::TransferID::GAMMA28: fn->fG = 2.8f; return true; - case ColorSpace::TransferID::BT709: - case ColorSpace::TransferID::SMPTE170M: - case ColorSpace::TransferID::BT2020_10: - case ColorSpace::TransferID::BT2020_12: - fn->fA = 0.909672431050f; - fn->fB = 0.090327568950f; - fn->fC = 0.222222222222f; - fn->fD = 0.081242862158f; - fn->fG = 2.222222222222f; - return true; case ColorSpace::TransferID::SMPTE240M: fn->fA = 0.899626676224f; fn->fB = 0.100373323776f; @@ -638,6 +717,19 @@ bool ColorSpace::GetTransferFunction(SkColorSpaceTransferFn* fn) const { fn->fD = 0.091286342118f; fn->fG = 2.222222222222f; return true; + case ColorSpace::TransferID::BT709: + case ColorSpace::TransferID::SMPTE170M: + case ColorSpace::TransferID::BT2020_10: + case ColorSpace::TransferID::BT2020_12: + // With respect to rendering BT709 + // * SMPTE 1886 suggests that we should be using gamma 2.4. + // * Most displays actually use a gamma of 2.2, and most media playing + // software uses the sRGB transfer function. + // * User studies shows that users don't really care. + // * Apple's CoreVideo uses gamma=1.961. + // Bearing all of that in mind, use the same transfer funciton as sRGB, + // which will allow more optimization, and will more closely match other + // media players. case ColorSpace::TransferID::IEC61966_2_1: case ColorSpace::TransferID::IEC61966_2_1_HDR: fn->fA = 0.947867345704f; diff --git a/chromium/ui/gfx/color_space.h b/chromium/ui/gfx/color_space.h index 61574a5fcb8..8a514de26be 100644 --- a/chromium/ui/gfx/color_space.h +++ b/chromium/ui/gfx/color_space.h @@ -7,6 +7,7 @@ #include <stdint.h> #include <ostream> +#include <vector> #include "base/gtest_prod_util.h" #include "base/macros.h" @@ -22,6 +23,7 @@ struct ParamTraits; namespace gfx { class ICCProfile; +class ICCProfileCache; // Used to represet a color space for the purpose of color conversion. // This is designed to be safe and compact enough to send over IPC @@ -124,6 +126,10 @@ class COLOR_SPACE_EXPORT ColorSpace { TransferID transfer, MatrixID matrix, RangeID full_range); + ColorSpace(PrimaryID primaries, + const SkColorSpaceTransferFn& fn, + MatrixID matrix, + RangeID full_range); ColorSpace(const ColorSpace& other); ColorSpace(ColorSpace&& other); ColorSpace& operator=(const ColorSpace& other); @@ -143,6 +149,7 @@ class COLOR_SPACE_EXPORT ColorSpace { // Extended sRGB matches sRGB for values in [0, 1], and extends the transfer // function to all real values. static ColorSpace CreateExtendedSRGB(); + // scRGB uses the same primaries as sRGB but has a linear transfer function // for all real values. static ColorSpace CreateSCRGBLinear(); @@ -160,18 +167,28 @@ class COLOR_SPACE_EXPORT ColorSpace { // Returns true if the decoded values can be outside of the 0.0-1.0 range. bool IsHDR() const; + // Returns true if the encoded values can be outside of the 0.0-1.0 range. bool FullRangeEncodedValues() const; // Returns true if this color space can be represented parametrically. bool IsParametric() const; + // Return a parametric approximation of this color space (if it is not already // parametric). - gfx::ColorSpace GetParametricApproximation() const; + ColorSpace GetParametricApproximation() const; // Return this color space with any range adjust or YUV to RGB conversion // stripped off. - gfx::ColorSpace GetAsFullRangeRGB() const; + ColorSpace GetAsFullRangeRGB() const; + + // If |this| is the final output color space, return the color space that + // would be appropriate for rasterization. + ColorSpace GetRasterColorSpace() const; + + // If |this| is the final output color space, return the color space that + // would be appropriate for blending. + ColorSpace GetBlendingColorSpace() const; // This will return nullptr for non-RGB spaces, spaces with non-FULL // range, and unspecified spaces. @@ -180,6 +197,7 @@ class COLOR_SPACE_EXPORT ColorSpace { // Populate |icc_profile| with an ICC profile that represents this color // space. Returns false if this space is not representable. bool GetICCProfile(ICCProfile* icc_profile) const; + bool GetICCProfileData(std::vector<char>* data) const; void GetPrimaryMatrix(SkMatrix44* to_XYZD50) const; bool GetTransferFunction(SkColorSpaceTransferFn* fn) const; @@ -190,6 +208,8 @@ class COLOR_SPACE_EXPORT ColorSpace { void GetRangeAdjustMatrix(SkMatrix44* matrix) const; private: + void SetCustomTransferFunction(const SkColorSpaceTransferFn& fn); + // Returns true if the transfer function is defined by an // SkColorSpaceTransferFn which is extended to all real values. bool HasExtendedSkTransferFn() const; @@ -213,10 +233,11 @@ class COLOR_SPACE_EXPORT ColorSpace { sk_sp<SkColorSpace> icc_profile_sk_color_space_; friend class ICCProfile; + friend class ICCProfileCache; friend class ColorTransform; friend class ColorTransformInternal; friend class ColorSpaceWin; - friend struct IPC::ParamTraits<gfx::ColorSpace>; + friend struct IPC::ParamTraits<ColorSpace>; FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, GetColorSpace); }; diff --git a/chromium/ui/gfx/color_space_unittest.cc b/chromium/ui/gfx/color_space_unittest.cc index 211604c14fb..bef60b8f80c 100644 --- a/chromium/ui/gfx/color_space_unittest.cc +++ b/chromium/ui/gfx/color_space_unittest.cc @@ -239,6 +239,23 @@ TEST(ColorSpace, ApproximateTransferFnBadMatch) { } } +TEST(ColorSpace, RasterAndBlend) { + ColorSpace display_color_space; + + // A linear transfer function being used for HDR should be blended using an + // sRGB-like transfer function. + display_color_space = ColorSpace::CreateSCRGBLinear(); + EXPECT_EQ(ColorSpace::CreateExtendedSRGB(), + display_color_space.GetBlendingColorSpace()); + EXPECT_EQ(ColorSpace::CreateDisplayP3D65(), + display_color_space.GetRasterColorSpace()); + + // If not used for HDR, a linear transfer function should be left unchanged. + display_color_space = ColorSpace::CreateXYZD50(); + EXPECT_EQ(display_color_space, display_color_space.GetBlendingColorSpace()); + EXPECT_EQ(display_color_space, display_color_space.GetRasterColorSpace()); +} + TEST(ColorSpace, ToSkColorSpace) { const size_t kNumTests = 4; ColorSpace color_spaces[kNumTests] = { diff --git a/chromium/ui/gfx/color_transform.cc b/chromium/ui/gfx/color_transform.cc index f3c3041fde6..3c31fb3f9f5 100644 --- a/chromium/ui/gfx/color_transform.cc +++ b/chromium/ui/gfx/color_transform.cc @@ -811,16 +811,6 @@ void ColorTransformInternal::AppendColorSpaceToColorSpaceTransform( ColorTransform::Intent intent) { if (intent == ColorTransform::Intent::INTENT_PERCEPTUAL) { switch (src.transfer_) { - case ColorSpace::TransferID::BT709: - case ColorSpace::TransferID::SMPTE170M: - // SMPTE 1886 suggests that we should be using gamma 2.4 for BT709 - // content. However, most displays actually use a gamma of 2.2, and - // user studies shows that users don't really care. Using the same - // gamma as the display will let us optimize a lot more, so lets stick - // with using the SRGB transfer function. - src.transfer_ = ColorSpace::TransferID::IEC61966_2_1; - break; - case ColorSpace::TransferID::SMPTEST2084: if (!dst.IsHDR()) { // We don't have an HDR display, so replace SMPTE 2084 with diff --git a/chromium/ui/gfx/color_transform_unittest.cc b/chromium/ui/gfx/color_transform_unittest.cc index d5217a77012..906adedceb8 100644 --- a/chromium/ui/gfx/color_transform_unittest.cc +++ b/chromium/ui/gfx/color_transform_unittest.cc @@ -133,6 +133,14 @@ TEST(SimpleColorSpace, TransferFnCancel) { ColorTransform::NewColorTransform( bt709, gamma24, ColorTransform::Intent::INTENT_PERCEPTUAL)); EXPECT_EQ(bt709_to_gamma24->NumberOfStepsForTesting(), 2u); + + // Rec 601 YUV to RGB conversion should have a single step. + gfx::ColorSpace rec601 = gfx::ColorSpace::CreateREC601(); + std::unique_ptr<ColorTransform> rec601_yuv_to_rgb( + ColorTransform::NewColorTransform( + rec601, rec601.GetAsFullRangeRGB(), + ColorTransform::Intent::INTENT_PERCEPTUAL)); + EXPECT_EQ(rec601_yuv_to_rgb->NumberOfStepsForTesting(), 1u); } TEST(SimpleColorSpace, SRGBFromICCAndNotICC) { diff --git a/chromium/ui/gfx/font_fallback_fuchsia.cc b/chromium/ui/gfx/font_fallback_fuchsia.cc new file mode 100644 index 00000000000..bfdc7aa2ce9 --- /dev/null +++ b/chromium/ui/gfx/font_fallback_fuchsia.cc @@ -0,0 +1,18 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/gfx/font_fallback.h" + +#include <string> +#include <vector> + +namespace gfx { + +std::vector<Font> GetFallbackFonts(const Font& font) { + // TODO(fuchsia): Stubbed while bringing up headless build, see + // https://crbug.com/743296. + return std::vector<Font>(); +} + +} // namespace gfx diff --git a/chromium/ui/gfx/font_render_params.h b/chromium/ui/gfx/font_render_params.h index ade3672f493..39d72e35203 100644 --- a/chromium/ui/gfx/font_render_params.h +++ b/chromium/ui/gfx/font_render_params.h @@ -113,8 +113,7 @@ GFX_EXPORT FontRenderParams GetFontRenderParams( GFX_EXPORT void ClearFontRenderParamsCacheForTest(); #endif -#if defined(OS_CHROMEOS) || defined(OS_LINUX) || \ - (defined(OS_ANDROID) && BUILDFLAG(ENABLE_VR)) +#if defined(OS_LINUX) || (defined(OS_ANDROID) && BUILDFLAG(ENABLE_VR)) // Gets the device scale factor to query the FontRenderParams. GFX_EXPORT float GetFontRenderParamsDeviceScaleFactor(); diff --git a/chromium/ui/gfx/font_render_params_fuchsia.cc b/chromium/ui/gfx/font_render_params_fuchsia.cc new file mode 100644 index 00000000000..f693f926c22 --- /dev/null +++ b/chromium/ui/gfx/font_render_params_fuchsia.cc @@ -0,0 +1,38 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/gfx/font_render_params.h" + +#include "base/logging.h" +#include "base/macros.h" + +namespace gfx { + +namespace { + +// Returns the system's default settings. +FontRenderParams LoadDefaults() { + FontRenderParams params; + params.antialiasing = true; + params.autohinter = true; + params.use_bitmaps = true; + params.subpixel_rendering = FontRenderParams::SUBPIXEL_RENDERING_NONE; + params.subpixel_positioning = true; + params.hinting = FontRenderParams::HINTING_SLIGHT; + + return params; +} + +} // namespace + +FontRenderParams GetFontRenderParams(const FontRenderParamsQuery& query, + std::string* family_out) { + if (family_out) + NOTIMPLEMENTED(); + // Customized font rendering settings are not supported, only defaults. + CR_DEFINE_STATIC_LOCAL(const gfx::FontRenderParams, params, (LoadDefaults())); + return params; +} + +} // namespace gfx diff --git a/chromium/ui/gfx/geometry/point.h b/chromium/ui/gfx/geometry/point.h index bb248d5432b..b1ba5065de4 100644 --- a/chromium/ui/gfx/geometry/point.h +++ b/chromium/ui/gfx/geometry/point.h @@ -9,7 +9,7 @@ #include <string> #include <tuple> -#include "base/numerics/saturated_arithmetic.h" +#include "base/numerics/clamped_math.h" #include "build/build_config.h" #include "ui/gfx/geometry/vector2d.h" #include "ui/gfx/gfx_export.h" @@ -56,18 +56,18 @@ class GFX_EXPORT Point { } void Offset(int delta_x, int delta_y) { - x_ = base::SaturatedAddition(x_, delta_x); - y_ = base::SaturatedAddition(y_, delta_y); + x_ = base::ClampAdd(x_, delta_x); + y_ = base::ClampAdd(y_, delta_y); } void operator+=(const Vector2d& vector) { - x_ = base::SaturatedAddition(x_, vector.x()); - y_ = base::SaturatedAddition(y_, vector.y()); + x_ = base::ClampAdd(x_, vector.x()); + y_ = base::ClampAdd(y_, vector.y()); } void operator-=(const Vector2d& vector) { - x_ = base::SaturatedSubtraction(x_, vector.x()); - y_ = base::SaturatedSubtraction(y_, vector.y()); + x_ = base::ClampSub(x_, vector.x()); + y_ = base::ClampSub(y_, vector.y()); } void SetToMin(const Point& other); @@ -116,8 +116,8 @@ inline Point operator-(const Point& lhs, const Vector2d& rhs) { } inline Vector2d operator-(const Point& lhs, const Point& rhs) { - return Vector2d(base::SaturatedSubtraction(lhs.x(), rhs.x()), - base::SaturatedSubtraction(lhs.y(), rhs.y())); + return Vector2d(base::ClampSub(lhs.x(), rhs.x()), + base::ClampSub(lhs.y(), rhs.y())); } inline Point PointAtOffsetFromOrigin(const Vector2d& offset_from_origin) { diff --git a/chromium/ui/gfx/geometry/rect.cc b/chromium/ui/gfx/geometry/rect.cc index b5ceda58291..63c0bbf04f0 100644 --- a/chromium/ui/gfx/geometry/rect.cc +++ b/chromium/ui/gfx/geometry/rect.cc @@ -15,7 +15,7 @@ #endif #include "base/logging.h" -#include "base/numerics/saturated_arithmetic.h" +#include "base/numerics/clamped_math.h" #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "ui/gfx/geometry/insets.h" @@ -69,8 +69,8 @@ static void SaturatedClampRange(int min, int max, int* origin, int* span) { return; } - int effective_span = base::SaturatedSubtraction(max, min); - int span_loss = base::SaturatedSubtraction(max, min + effective_span); + int effective_span = base::ClampSub(max, min); + int span_loss = base::ClampSub(max, min + effective_span); // If the desired width is within the limits of ints, we can just // use the simple computations to represent the range precisely. @@ -83,12 +83,12 @@ static void SaturatedClampRange(int min, int max, int* origin, int* span) { // Now we have to approximate. If one of min or max is close enough // to zero we choose to represent that one precisely. The other side is // probably practically "infinite", so we move it. - if (base::SaturatedAbsolute(max) < std::numeric_limits<int>::max() / 2) { + constexpr unsigned kMaxDimension = std::numeric_limits<int>::max() / 2; + if (base::SafeUnsignedAbs(max) < kMaxDimension) { // Maintain origin + span == max. *span = effective_span; *origin = max - effective_span; - } else if (base::SaturatedAbsolute(min) < - std::numeric_limits<int>::max() / 2) { + } else if (base::SafeUnsignedAbs(min) < kMaxDimension) { // Maintain origin == min. *span = effective_span; *origin = min; @@ -116,10 +116,8 @@ void Rect::Inset(int left, int top, int right, int bottom) { origin_ += Vector2d(left, top); // left+right might overflow/underflow, but width() - (left+right) might // overflow as well. - set_width(base::SaturatedSubtraction(width(), - base::SaturatedAddition(left, right))); - set_height(base::SaturatedSubtraction(height(), - base::SaturatedAddition(top, bottom))); + set_width(base::ClampSub(width(), base::ClampAdd(left, right))); + set_height(base::ClampSub(height(), base::ClampAdd(top, bottom))); } void Rect::Offset(int horizontal, int vertical) { diff --git a/chromium/ui/gfx/geometry/rect.h b/chromium/ui/gfx/geometry/rect.h index 1858d44d2c7..c33d4a7bae6 100644 --- a/chromium/ui/gfx/geometry/rect.h +++ b/chromium/ui/gfx/geometry/rect.h @@ -227,7 +227,7 @@ class GFX_EXPORT Rect { // Clamp the size to avoid integer overflow in bottom() and right(). // This returns the width given an origin and a width. - // TODO(enne): this should probably use base::SaturatedAddition, but that + // TODO(enne): this should probably use base::ClampAdd, but that // function is not a constexpr. static constexpr int GetClampedValue(int origin, int size) { return AddWouldOverflow(origin, size) @@ -340,6 +340,41 @@ inline Rect ScaleToEnclosedRect(const Rect& rect, float scale) { return ScaleToEnclosedRect(rect, scale, scale); } +// Scales |rect| by scaling its four corner points. If the corner points lie on +// non-integral coordinate after scaling, their values are rounded to the +// nearest integer. +// This is helpful during layout when relative positions of multiple gfx::Rect +// in a given coordinate space needs to be same after scaling as it was before +// scaling. ie. this gives a lossless relative positioning of rects. +inline Rect ScaleToRoundedRect(const Rect& rect, float x_scale, float y_scale) { + if (x_scale == 1.f && y_scale == 1.f) + return rect; + + DCHECK( + base::IsValueInRangeForNumericType<int>(std::round(rect.x() * x_scale))); + DCHECK( + base::IsValueInRangeForNumericType<int>(std::round(rect.y() * y_scale))); + DCHECK(base::IsValueInRangeForNumericType<int>( + std::round(rect.right() * x_scale))); + DCHECK(base::IsValueInRangeForNumericType<int>( + std::round(rect.bottom() * y_scale))); + + int x = static_cast<int>(std::round(rect.x() * x_scale)); + int y = static_cast<int>(std::round(rect.y() * y_scale)); + int r = rect.width() == 0 + ? x + : static_cast<int>(std::round(rect.right() * x_scale)); + int b = rect.height() == 0 + ? y + : static_cast<int>(std::round(rect.bottom() * y_scale)); + + return Rect(x, y, r - x, b - y); +} + +inline Rect ScaleToRoundedRect(const Rect& rect, float scale) { + return ScaleToRoundedRect(rect, scale, scale); +} + // This is declared here for use in gtest-based unit tests but is defined in // the //ui/gfx:test_support target. Depend on that to use this in your unit // test. This should not be used in production code - call ToString() instead. diff --git a/chromium/ui/gfx/geometry/size.cc b/chromium/ui/gfx/geometry/size.cc index 69486723a1d..781b84d7f51 100644 --- a/chromium/ui/gfx/geometry/size.cc +++ b/chromium/ui/gfx/geometry/size.cc @@ -12,8 +12,8 @@ #include <ApplicationServices/ApplicationServices.h> #endif +#include "base/numerics/clamped_math.h" #include "base/numerics/safe_math.h" -#include "base/numerics/saturated_arithmetic.h" #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "ui/gfx/geometry/safe_integer_conversions.h" @@ -58,8 +58,8 @@ base::CheckedNumeric<int> Size::GetCheckedArea() const { } void Size::Enlarge(int grow_width, int grow_height) { - SetSize(base::SaturatedAddition(width(), grow_width), - base::SaturatedAddition(height(), grow_height)); + SetSize(base::ClampAdd(width(), grow_width), + base::ClampAdd(height(), grow_height)); } void Size::SetToMin(const Size& other) { diff --git a/chromium/ui/gfx/geometry/vector2d.cc b/chromium/ui/gfx/geometry/vector2d.cc index 2b4875c39cb..0ce3b20baa5 100644 --- a/chromium/ui/gfx/geometry/vector2d.cc +++ b/chromium/ui/gfx/geometry/vector2d.cc @@ -6,7 +6,7 @@ #include <cmath> -#include "base/numerics/saturated_arithmetic.h" +#include "base/numerics/clamped_math.h" #include "base/strings/stringprintf.h" namespace gfx { @@ -16,13 +16,13 @@ bool Vector2d::IsZero() const { } void Vector2d::Add(const Vector2d& other) { - x_ = base::SaturatedAddition(other.x_, x_); - y_ = base::SaturatedAddition(other.y_, y_); + x_ = base::ClampAdd(other.x_, x_); + y_ = base::ClampAdd(other.y_, y_); } void Vector2d::Subtract(const Vector2d& other) { - x_ = base::SaturatedSubtraction(x_, other.x_); - y_ = base::SaturatedSubtraction(y_, other.y_); + x_ = base::ClampSub(x_, other.x_); + y_ = base::ClampSub(y_, other.y_); } int64_t Vector2d::LengthSquared() const { diff --git a/chromium/ui/gfx/gpu_memory_buffer_tracing.cc b/chromium/ui/gfx/gpu_memory_buffer_tracing.cc deleted file mode 100644 index 5f587074862..00000000000 --- a/chromium/ui/gfx/gpu_memory_buffer_tracing.cc +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "ui/gfx/gpu_memory_buffer_tracing.h" - -#include "base/format_macros.h" -#include "base/strings/stringprintf.h" - -namespace gfx { - -base::trace_event::MemoryAllocatorDumpGuid GetSharedMemoryGUIDForTracing( - uint64_t tracing_process_id, - GpuMemoryBufferId buffer_id) { - // TODO(hajimehoshi): This should be unified to shared memory GUIDs in - // base/memory/shared_memory_tracker.cc - return base::trace_event::MemoryAllocatorDumpGuid(base::StringPrintf( - "shared_memory_gpu/%" PRIx64 "/%d", tracing_process_id, buffer_id.id)); -} - -} // namespace gfx diff --git a/chromium/ui/gfx/gpu_memory_buffer_tracing.h b/chromium/ui/gfx/gpu_memory_buffer_tracing.h deleted file mode 100644 index 0b66a91abd8..00000000000 --- a/chromium/ui/gfx/gpu_memory_buffer_tracing.h +++ /dev/null @@ -1,15 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/trace_event/memory_allocator_dump_guid.h" -#include "ui/gfx/gfx_export.h" -#include "ui/gfx/gpu_memory_buffer.h" - -namespace gfx { - -base::trace_event::MemoryAllocatorDumpGuid GFX_EXPORT -GetSharedMemoryGUIDForTracing(uint64_t tracing_process_id, - GpuMemoryBufferId buffer_id); - -} // namespace gfx diff --git a/chromium/ui/gfx/icc_profile.cc b/chromium/ui/gfx/icc_profile.cc index a982db54eff..0a9517ac7bd 100644 --- a/chromium/ui/gfx/icc_profile.cc +++ b/chromium/ui/gfx/icc_profile.cc @@ -5,10 +5,12 @@ #include "ui/gfx/icc_profile.h" #include <list> +#include <set> #include "base/command_line.h" #include "base/containers/mru_cache.h" #include "base/lazy_instance.h" +#include "base/metrics/histogram_macros.h" #include "base/synchronization/lock.h" #include "third_party/skia/include/core/SkColorSpaceXform.h" #include "third_party/skia/include/core/SkICC.h" @@ -21,36 +23,194 @@ const uint64_t ICCProfile::test_id_adobe_rgb_ = 1; const uint64_t ICCProfile::test_id_color_spin_ = 2; const uint64_t ICCProfile::test_id_generic_rgb_ = 3; const uint64_t ICCProfile::test_id_srgb_ = 4; -const uint64_t ICCProfile::test_id_no_analytic_tr_fn_ = 5; -const uint64_t ICCProfile::test_id_a2b_only_ = 6; -const uint64_t ICCProfile::test_id_overshoot_ = 7; -namespace { +// A MRU cache of ICC profiles. The cache key is a uin64_t which a +// gfx::ColorSpace may use to refer back to an ICC profile in the cache. The +// data cached for each profile is the gfx::ICCProfile structure (which includes +// the associated gfx::ColorSpace approximations and SkColorSpace structures) +// and whether or not the ICC profile has been histogrammed. +class ICCProfileCache { + public: + // Allow keeping around a maximum of 16 cached ICC profiles. Beware that + // we will do a linear search thorugh currently-cached ICC profiles, + // when creating a new ICC profile. + static const size_t kMaxCachedICCProfiles = 16; + + ICCProfileCache() : id_to_icc_profile_mru_(kMaxCachedICCProfiles) {} + ~ICCProfileCache() {} + + // Add |icc_profile| to the cache. If |icc_profile| does not have an id set + // yet, assign an id to it. + void InsertAndSetIdIfNeeded(ICCProfile* icc_profile) { + base::AutoLock lock(lock_); + + if (FindByIdUnderLock(icc_profile->id_, icc_profile)) + return; + + if (FindByDataUnderLock(icc_profile->data_.data(), + icc_profile->data_.size(), icc_profile)) { + return; + } + + if (!icc_profile->id_) + icc_profile->id_ = next_unused_id_++; + + // Ensure that GetColorSpace() point back to this ICCProfile. + gfx::ColorSpace& color_space = icc_profile->color_space_; + color_space.icc_profile_id_ = icc_profile->id_; + + // Ensure that the GetParametricColorSpace() point back to this ICCProfile + // only if the parametric version is accurate. + if (color_space.primaries_ != ColorSpace::PrimaryID::ICC_BASED && + color_space.transfer_ != ColorSpace::TransferID::ICC_BASED) { + icc_profile->parametric_color_space_.icc_profile_id_ = icc_profile->id_; + } + + Entry entry; + entry.icc_profile = *icc_profile; + id_to_icc_profile_mru_.Put(icc_profile->id_, entry); + } + + // We maintain UMA histograms of display ICC profiles. Only histogram a + // display once for each |display_id| (because we will re-read the same + // ICC profile repeatedly when reading other display profiles, which will + // skew samples). Return true if we need to histogram this profile for + // |display_id|, and ensure that all future calls will return false for + // |display_id|. + bool GetAndSetNeedsHistogram(uint64_t display_id, + const ICCProfile& icc_profile) { + base::AutoLock lock(lock_); + + // If we don't find the profile in the cache, don't histogram it. + auto found = id_to_icc_profile_mru_.Get(icc_profile.id_); + if (found == id_to_icc_profile_mru_.end()) + return false; + + // If we have already histogrammed this display, don't histogram it. + std::set<int64_t>& histogrammed_display_ids = + found->second.histogrammed_display_ids; + if (histogrammed_display_ids.count(display_id)) + return false; + + // Histogram this display, and mark that we have done so. + histogrammed_display_ids.insert(display_id); + return true; + } + + // Move this ICC profile to the most recently used end of the cache, + // re-inserting if needed. + void TouchEntry(const ICCProfile& icc_profile) { + base::AutoLock lock(lock_); + + if (!icc_profile.id_) + return; + + // Look up the profile by id to move it to the front of the MRU. + auto found = id_to_icc_profile_mru_.Get(icc_profile.id_); + if (found != id_to_icc_profile_mru_.end()) + return; + + // Look up the profile by its data. If there is a new entry for the same + // data, don't add a duplicate. + if (FindByDataUnderLock(icc_profile.data_.data(), icc_profile.data_.size(), + nullptr)) { + return; + } -// Allow keeping around a maximum of 8 cached ICC profiles. Beware that -// we will do a linear search thorugh currently-cached ICC profiles, -// when creating a new ICC profile. -const size_t kMaxCachedICCProfiles = 8; + // If the entry was not found, insert it. + Entry entry; + entry.icc_profile = icc_profile; + id_to_icc_profile_mru_.Put(icc_profile.id_, entry); + } -struct Cache { - Cache() : id_to_icc_profile_mru(kMaxCachedICCProfiles) {} - ~Cache() {} + // Look up an ICC profile in the cache by its data (to ensure that the same + // data gets the same id every time). On success, return true and populate + // |icc_profile| with the associated profile. + bool FindByData(const void* data, size_t size, ICCProfile* icc_profile) { + base::AutoLock lock(lock_); + return FindByDataUnderLock(data, size, icc_profile); + } + + // Look up an ICC profile in the cache by its id. On success, return true and + // populate |icc_profile| with the associated profile. + bool FindById(uint64_t id, ICCProfile* icc_profile) { + base::AutoLock lock(lock_); + return FindByIdUnderLock(id, icc_profile); + } + + private: + struct Entry { + ICCProfile icc_profile; + + // The set of display ids which have have caused this ICC profile to be + // recorded in UMA histograms. Only record an ICC profile once per display + // id (since the same profile will be re-read repeatedly, e.g, when displays + // are resized). + std::set<int64_t> histogrammed_display_ids; + }; + + // Body for FindById, executed when the cache lock is already held. + bool FindByIdUnderLock(uint64_t id, ICCProfile* icc_profile) { + lock_.AssertAcquired(); + if (!id) + return false; + + auto found = id_to_icc_profile_mru_.Get(id); + if (found == id_to_icc_profile_mru_.end()) + return false; + + *icc_profile = found->second.icc_profile; + return true; + } + + // Body for FindByData, executed when the cache lock is already held. + bool FindByDataUnderLock(const void* data, + size_t size, + ICCProfile* icc_profile) { + lock_.AssertAcquired(); + if (size == 0) + return false; + + for (const auto& id_entry_pair : id_to_icc_profile_mru_) { + const ICCProfile& cached_profile = id_entry_pair.second.icc_profile; + const std::vector<char>& iter_data = cached_profile.data_; + if (iter_data.size() != size || memcmp(data, iter_data.data(), size)) + continue; + + if (icc_profile) { + *icc_profile = cached_profile; + id_to_icc_profile_mru_.Get(cached_profile.id_); + } + return true; + } + return false; + } // Start from-ICC-data IDs at the end of the hard-coded test id list above. - uint64_t next_unused_id = 10; - base::MRUCache<uint64_t, ICCProfile> id_to_icc_profile_mru; - base::Lock lock; + uint64_t next_unused_id_ = 10; + base::MRUCache<uint64_t, Entry> id_to_icc_profile_mru_; + + // Lock that must be held to access |id_to_icc_profile_mru_| and + // |next_unused_id_|. + base::Lock lock_; }; -static base::LazyInstance<Cache>::DestructorAtExit g_cache = + +namespace { + +static base::LazyInstance<ICCProfileCache>::DestructorAtExit g_cache = LAZY_INSTANCE_INITIALIZER; -void ExtractColorSpaces(const std::vector<char>& data, - gfx::ColorSpace* parametric_color_space, - bool* parametric_color_space_is_accurate, - sk_sp<SkColorSpace>* useable_sk_color_space) { +} // namespace + +// static +ICCProfile::AnalyzeResult ICCProfile::ExtractColorSpaces( + const std::vector<char>& data, + gfx::ColorSpace* parametric_color_space, + float* parametric_tr_fn_max_error, + sk_sp<SkColorSpace>* useable_sk_color_space) { // Initialize the output parameters as invalid. *parametric_color_space = gfx::ColorSpace(); - *parametric_color_space_is_accurate = false; + *parametric_tr_fn_max_error = 0; *useable_sk_color_space = nullptr; // Parse the profile and attempt to create a SkColorSpaceXform out of it. @@ -58,20 +218,20 @@ void ExtractColorSpaces(const std::vector<char>& data, sk_sp<SkICC> sk_icc = SkICC::Make(data.data(), data.size()); if (!sk_icc) { DLOG(ERROR) << "Failed to parse ICC profile to SkICC."; - return; + return kICCFailedToParse; } sk_sp<SkColorSpace> sk_icc_color_space = SkColorSpace::MakeICC(data.data(), data.size()); if (!sk_icc_color_space) { DLOG(ERROR) << "Failed to parse ICC profile to SkColorSpace."; - return; + return kICCFailedToExtractSkColorSpace; } std::unique_ptr<SkColorSpaceXform> sk_color_space_xform = SkColorSpaceXform::New(sk_srgb_color_space.get(), sk_icc_color_space.get()); if (!sk_color_space_xform) { DLOG(ERROR) << "Parsed ICC profile but can't create SkColorSpaceXform."; - return; + return kICCFailedToCreateXform; } // Because this SkColorSpace can be used to construct a transform, mark it @@ -82,15 +242,14 @@ void ExtractColorSpaces(const std::vector<char>& data, // If our SkColorSpace representation is sRGB then return that. if (SkColorSpace::Equals(sk_srgb_color_space.get(), sk_icc_color_space.get())) { - *parametric_color_space_is_accurate = true; - return; + return kICCExtractedSRGBColorSpace; } // A primary matrix is required for our parametric approximation. SkMatrix44 to_XYZD50_matrix; if (!sk_icc->toXYZD50(&to_XYZD50_matrix)) { DLOG(ERROR) << "Failed to extract ICC profile primary matrix."; - return; + return kICCFailedToExtractMatrix; } // Try to directly extract a numerical transfer function. @@ -98,27 +257,7 @@ void ExtractColorSpaces(const std::vector<char>& data, if (sk_icc->isNumericalTransferFn(&exact_tr_fn)) { *parametric_color_space = gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, exact_tr_fn); - *parametric_color_space_is_accurate = true; - return; - } - - // If that fails, try to numerically approximate the transfer function. - SkColorSpaceTransferFn approx_tr_fn; - float approx_tr_fn_max_error = 0; - if (SkApproximateTransferFn(sk_icc, &approx_tr_fn_max_error, &approx_tr_fn)) { - const float kMaxError = 2.f / 256.f; - if (approx_tr_fn_max_error < kMaxError) { - *parametric_color_space = - gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, approx_tr_fn); - *parametric_color_space_is_accurate = true; - return; - } else { - DLOG(ERROR) - << "Failed to accurately approximate transfer function, error: " - << 256.f * approx_tr_fn_max_error << "/256"; - } - } else { - DLOG(ERROR) << "Failed approximate transfer function."; + return kICCExtractedMatrixAndAnalyticTrFn; } // If we fail to get a transfer function, use the sRGB transfer function, @@ -127,9 +266,31 @@ void ExtractColorSpaces(const std::vector<char>& data, // SkColorSpace. *parametric_color_space = gfx::ColorSpace::CreateCustom( to_XYZD50_matrix, ColorSpace::TransferID::IEC61966_2_1); -} -} // namespace + // Attempt to fit a parametric transfer function to the table data in the + // profile. + SkColorSpaceTransferFn approx_tr_fn; + if (!SkApproximateTransferFn(sk_icc, parametric_tr_fn_max_error, + &approx_tr_fn)) { + DLOG(ERROR) << "Failed approximate transfer function."; + return kICCFailedToConvergeToApproximateTrFn; + } + + // If this converged, but has too high error, use the sRGB transfer function + // from above. + const float kMaxError = 2.f / 256.f; + if (*parametric_tr_fn_max_error >= kMaxError) { + DLOG(ERROR) << "Failed to accurately approximate transfer function, error: " + << 256.f * (*parametric_tr_fn_max_error) << "/256"; + return kICCFailedToApproximateTrFnAccurately; + }; + + // If the error is sufficiently low, declare that the approximation is + // accurate. + *parametric_color_space = + gfx::ColorSpace::CreateCustom(to_XYZD50_matrix, approx_tr_fn); + return kICCExtractedMatrixAndApproximatedTrFn; +} ICCProfile::ICCProfile() = default; ICCProfile::ICCProfile(ICCProfile&& other) = default; @@ -147,7 +308,15 @@ bool ICCProfile::operator!=(const ICCProfile& other) const { } bool ICCProfile::IsValid() const { - return successfully_parsed_by_sk_icc_; + switch (analyze_result_) { + case kICCFailedToParse: + case kICCFailedToExtractSkColorSpace: + case kICCFailedToCreateXform: + return false; + default: + break; + } + return true; } // static @@ -159,30 +328,14 @@ ICCProfile ICCProfile::FromData(const void* data, size_t size) { ICCProfile ICCProfile::FromDataWithId(const void* data, size_t size, uint64_t new_profile_id) { - if (!size) - return ICCProfile(); + ICCProfile icc_profile; - const char* data_as_char = reinterpret_cast<const char*>(data); - { - // Linearly search the cached ICC profiles to find one with the same data. - // If it exists, re-use its id and touch it in the cache. - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - for (auto iter = cache.id_to_icc_profile_mru.begin(); - iter != cache.id_to_icc_profile_mru.end(); ++iter) { - const std::vector<char>& iter_data = iter->second.data_; - if (iter_data.size() != size || memcmp(data, iter_data.data(), size)) - continue; - auto found = cache.id_to_icc_profile_mru.Get(iter->second.id_); - return found->second; - } - if (!new_profile_id) - new_profile_id = cache.next_unused_id++; - } + if (!size) + return icc_profile; // Create a new cached id and add it to the cache. - ICCProfile icc_profile; icc_profile.id_ = new_profile_id; + const char* data_as_char = reinterpret_cast<const char*>(data); icc_profile.data_.insert(icc_profile.data_.begin(), data_as_char, data_as_char + size); icc_profile.ComputeColorSpaceAndCache(); @@ -203,93 +356,89 @@ const std::vector<char>& ICCProfile::GetData() const { } const ColorSpace& ICCProfile::GetColorSpace() const { - // Move this ICC profile to the most recently used end of the cache, - // inserting if needed. - if (id_) { - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - auto found = cache.id_to_icc_profile_mru.Get(id_); - if (found == cache.id_to_icc_profile_mru.end()) - found = cache.id_to_icc_profile_mru.Put(id_, *this); - } + g_cache.Get().TouchEntry(*this); return color_space_; } const ColorSpace& ICCProfile::GetParametricColorSpace() const { - // Move this ICC profile to the most recently used end of the cache, - // inserting if needed. - if (id_) { - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - auto found = cache.id_to_icc_profile_mru.Get(id_); - if (found == cache.id_to_icc_profile_mru.end()) - found = cache.id_to_icc_profile_mru.Put(id_, *this); - } + g_cache.Get().TouchEntry(*this); return parametric_color_space_; } // static bool ICCProfile::FromId(uint64_t id, ICCProfile* icc_profile) { - if (!id) - return false; - - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - - auto found = cache.id_to_icc_profile_mru.Get(id); - if (found == cache.id_to_icc_profile_mru.end()) - return false; - - *icc_profile = found->second; - return true; + return g_cache.Get().FindById(id, icc_profile); } void ICCProfile::ComputeColorSpaceAndCache() { - if (!id_) + // Early out for empty entries. + if (data_.empty()) return; - // If this already exists in the cache, just update its |color_space_|. - { - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - auto found = cache.id_to_icc_profile_mru.Get(id_); - if (found != cache.id_to_icc_profile_mru.end()) { - color_space_ = found->second.color_space_; - parametric_color_space_ = found->second.parametric_color_space_; - successfully_parsed_by_sk_icc_ = - found->second.successfully_parsed_by_sk_icc_; - return; - } - } + // If this id already exists in the cache, copy |this| from the cache entry. + if (g_cache.Get().FindById(id_, this)) + return; + + // If this data already exists in the cache, copy |this| from the cache entry. + if (g_cache.Get().FindByData(data_.data(), data_.size(), this)) + return; // Parse the ICC profile sk_sp<SkColorSpace> useable_sk_color_space; - bool parametric_color_space_is_accurate = false; - ExtractColorSpaces(data_, ¶metric_color_space_, - ¶metric_color_space_is_accurate, - &useable_sk_color_space); - if (parametric_color_space_is_accurate) { - successfully_parsed_by_sk_icc_ = true; - parametric_color_space_.icc_profile_id_ = id_; - color_space_ = parametric_color_space_; - } else if (useable_sk_color_space) { - successfully_parsed_by_sk_icc_ = true; - color_space_ = ColorSpace(ColorSpace::PrimaryID::ICC_BASED, - ColorSpace::TransferID::ICC_BASED); - color_space_.icc_profile_id_ = id_; - color_space_.icc_profile_sk_color_space_ = useable_sk_color_space; - } else { - successfully_parsed_by_sk_icc_ = false; - DCHECK(!color_space_.IsValid()); - color_space_ = parametric_color_space_; + analyze_result_ = + ExtractColorSpaces(data_, ¶metric_color_space_, + ¶metric_tr_fn_error_, &useable_sk_color_space); + switch (analyze_result_) { + case kICCExtractedSRGBColorSpace: + case kICCExtractedMatrixAndAnalyticTrFn: + case kICCExtractedMatrixAndApproximatedTrFn: + // Successfully and accurately extracted color space. + color_space_ = parametric_color_space_; + break; + case kICCFailedToExtractRawTrFn: + case kICCFailedToExtractMatrix: + case kICCFailedToConvergeToApproximateTrFn: + case kICCFailedToApproximateTrFnAccurately: + // Successfully but extracted a color space, but it isn't accurate enough. + color_space_ = ColorSpace(ColorSpace::PrimaryID::ICC_BASED, + ColorSpace::TransferID::ICC_BASED); + color_space_.icc_profile_sk_color_space_ = useable_sk_color_space; + break; + case kICCFailedToParse: + case kICCFailedToExtractSkColorSpace: + case kICCFailedToCreateXform: + // Can't even use this color space as a LUT. + DCHECK(!parametric_color_space_.IsValid()); + color_space_ = parametric_color_space_; + break; } // Add to the cache. - { - Cache& cache = g_cache.Get(); - base::AutoLock lock(cache.lock); - cache.id_to_icc_profile_mru.Put(id_, *this); + g_cache.Get().InsertAndSetIdIfNeeded(this); +} + +void ICCProfile::HistogramDisplay(int64_t display_id) const { + if (!g_cache.Get().GetAndSetNeedsHistogram(display_id, *this)) + return; + + UMA_HISTOGRAM_ENUMERATION("Blink.ColorSpace.Destination.ICCResult", + analyze_result_, kICCProfileAnalyzeLast); + + // Add histograms for numerical approximation. + bool nonlinear_fit_converged = + analyze_result_ == kICCExtractedMatrixAndApproximatedTrFn || + analyze_result_ == kICCFailedToApproximateTrFnAccurately; + bool nonlinear_fit_did_not_converge = + analyze_result_ == kICCFailedToConvergeToApproximateTrFn; + if (nonlinear_fit_converged || nonlinear_fit_did_not_converge) { + UMA_HISTOGRAM_BOOLEAN("Blink.ColorSpace.Destination.NonlinearFitConverged", + nonlinear_fit_converged); + } + if (nonlinear_fit_converged) { + UMA_HISTOGRAM_CUSTOM_COUNTS( + "Blink.ColorSpace.Destination.NonlinearFitError", + static_cast<int>(parametric_tr_fn_error_ * 255), 0, 127, 16); } } diff --git a/chromium/ui/gfx/icc_profile.h b/chromium/ui/gfx/icc_profile.h index 03170adfbda..257b536dba3 100644 --- a/chromium/ui/gfx/icc_profile.h +++ b/chromium/ui/gfx/icc_profile.h @@ -23,6 +23,8 @@ template <typename, typename> struct StructTraits; namespace gfx { +class ICCProfileCache; + // Used to represent a full ICC profile, usually retrieved from a monitor. It // can be lossily compressed into a ColorSpace object. This structure should // only be sent from higher-privilege processes to lower-privilege processes, @@ -49,11 +51,6 @@ class COLOR_SPACE_EXPORT ICCProfile { static ICCProfile FromCGColorSpace(CGColorSpaceRef cg_color_space); #endif - // This will recover a ICCProfile from a compact ColorSpace representation. - // Internally, this will make an effort to create an identical ICCProfile - // to the one that created |color_space|, but this is not guaranteed. - static ICCProfile FromColorSpace(const gfx::ColorSpace& color_space); - // Create directly from profile data. static ICCProfile FromData(const void* icc_profile, size_t size); @@ -68,6 +65,10 @@ class COLOR_SPACE_EXPORT ICCProfile { const std::vector<char>& GetData() const; + // Histogram how we this was approximated by a gfx::ColorSpace. Only + // histogram a given profile once per display. + void HistogramDisplay(int64_t display_id) const; + #if defined(OS_WIN) // This will read monitor ICC profiles from disk and cache the results for the // other functions to read. This should not be called on the UI or IO thread. @@ -76,33 +77,49 @@ class COLOR_SPACE_EXPORT ICCProfile { #endif private: + // This must match ICCProfileAnalyzeResult enum in histograms.xml. + enum AnalyzeResult { + kICCExtractedMatrixAndAnalyticTrFn = 0, + kICCExtractedMatrixAndApproximatedTrFn = 1, + kICCFailedToConvergeToApproximateTrFn = 2, + kICCFailedToExtractRawTrFn = 3, + kICCFailedToExtractMatrix = 4, + kICCFailedToParse = 5, + kICCFailedToExtractSkColorSpace = 6, + kICCFailedToCreateXform = 7, + kICCFailedToApproximateTrFnAccurately = 8, + kICCExtractedSRGBColorSpace = 9, + kICCProfileAnalyzeLast = kICCExtractedSRGBColorSpace, + }; + + friend class ICCProfileCache; friend ICCProfile ICCProfileForTestingAdobeRGB(); friend ICCProfile ICCProfileForTestingColorSpin(); friend ICCProfile ICCProfileForTestingGenericRGB(); friend ICCProfile ICCProfileForTestingSRGB(); - friend ICCProfile ICCProfileForTestingNoAnalyticTrFn(); - friend ICCProfile ICCProfileForTestingA2BOnly(); - friend ICCProfile ICCProfileForTestingOvershoot(); friend ICCProfile ICCProfileForLayoutTests(); static const uint64_t test_id_adobe_rgb_; static const uint64_t test_id_color_spin_; static const uint64_t test_id_generic_rgb_; static const uint64_t test_id_srgb_; - static const uint64_t test_id_no_analytic_tr_fn_; - static const uint64_t test_id_a2b_only_; - static const uint64_t test_id_overshoot_; // Populate |icc_profile| with the ICCProfile corresponding to id |id|. Return // false if |id| is not in the cache. static bool FromId(uint64_t id, ICCProfile* icc_profile); // This method is used to hard-code the |id_| to a specific value, and is - // used by test methods to ensure that they don't conflict with the values - // generated in the browser. + // used by layout test methods to ensure that they don't conflict with the + // values generated in the browser. static ICCProfile FromDataWithId(const void* icc_profile, size_t size, uint64_t id); + static AnalyzeResult ExtractColorSpaces( + const std::vector<char>& data, + gfx::ColorSpace* parametric_color_space, + float* parametric_tr_fn_max_error, + sk_sp<SkColorSpace>* useable_sk_color_space); + void ComputeColorSpaceAndCache(); // This globally identifies this ICC profile. It is used to look up this ICC @@ -111,6 +128,9 @@ class COLOR_SPACE_EXPORT ICCProfile { uint64_t id_ = 0; std::vector<char> data_; + // The result of attepting to extract a color space from the color profile. + AnalyzeResult analyze_result_ = kICCFailedToParse; + // |color_space| always links back to this ICC profile, and its SkColorSpace // is always equal to the SkColorSpace created from this ICCProfile. gfx::ColorSpace color_space_; @@ -119,8 +139,10 @@ class COLOR_SPACE_EXPORT ICCProfile { // is accurate, and its SkColorSpace will always be parametrically created. gfx::ColorSpace parametric_color_space_; - // This is set to true if SkICC successfully parsed this profile. - bool successfully_parsed_by_sk_icc_ = false; + // The L-infinity error of the parametric color space fit. This is undefined + // unless |analyze_result_| is kICCFailedToApproximateTrFnAccurately or + // kICCExtractedMatrixAndApproximatedTrFn. + float parametric_tr_fn_error_ = -1; FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, BT709toSRGBICC); FRIEND_TEST_ALL_PREFIXES(SimpleColorSpace, GetColorSpace); diff --git a/chromium/ui/gfx/icc_profile_unittest.cc b/chromium/ui/gfx/icc_profile_unittest.cc index e1222d5a3af..d9a008d71cd 100644 --- a/chromium/ui/gfx/icc_profile_unittest.cc +++ b/chromium/ui/gfx/icc_profile_unittest.cc @@ -122,6 +122,9 @@ TEST(ICCProfile, ParametricVersusExact) { // as invalid. ICCProfile a2b = ICCProfileForTestingA2BOnly(); EXPECT_FALSE(a2b.GetColorSpace().IsValid()); + + // Even though it is invalid, it should not be equal to the empty constructor. + EXPECT_NE(a2b, gfx::ICCProfile()); } TEST(ICCProfile, GarbageData) { @@ -158,4 +161,97 @@ TEST(ICCProfile, GenericRGB) { EXPECT_TRUE(SkMatrixIsApproximatelyIdentity(eye)); } +// This tests the ICCProfile MRU cache. This cache is sloppy and should be +// rewritten, now that some of the original design constraints have been lifted. +// This test exists only to ensure that we are aware of behavior changes, not to +// enforce that behavior does not change. +// https://crbug.com/766736 +TEST(ICCProfile, ExhaustCache) { + // Get an ICCProfile that can't be parametrically approximated. + ICCProfile original = ICCProfileForTestingNoAnalyticTrFn(); + ColorSpace original_color_space_0 = original.GetColorSpace(); + + // Recover the ICCProfile from its GetColorSpace. Recovery should succeed, and + // the ICCProfiles should be equal. + ICCProfile recovered_0; + EXPECT_TRUE(original_color_space_0.GetICCProfile(&recovered_0)); + EXPECT_EQ(original, recovered_0); + + // The GetColorSpace of the recovered version should match the original. + ColorSpace recovered_0_color_space = recovered_0.GetColorSpace(); + EXPECT_EQ(recovered_0_color_space, original_color_space_0); + + // Create an identical ICCProfile to the original. It should equal the + // original, and its GetColorSpace should equal the original. + ICCProfile identical_0 = ICCProfileForTestingNoAnalyticTrFn(); + EXPECT_EQ(original, identical_0); + ColorSpace identical_color_space_0 = identical_0.GetColorSpace(); + EXPECT_EQ(identical_color_space_0, original_color_space_0); + + // Create 128 distinct ICC profiles. This will destroy the cached + // ICCProfile<->ColorSpace mapping. + for (size_t i = 0; i < 128; ++i) { + SkMatrix44 toXYZD50; + ColorSpace::CreateSRGB().GetPrimaryMatrix(&toXYZD50); + SkColorSpaceTransferFn fn; + fn.fA = 1; + fn.fB = 0; + fn.fC = 1; + fn.fD = 0; + fn.fE = 0; + fn.fF = 0; + fn.fG = 1.5f + i / 128.f; + ColorSpace color_space = ColorSpace::CreateCustom(toXYZD50, fn); + ICCProfile icc_profile; + color_space.GetICCProfile(&icc_profile); + } + + // Recover the original ICCProfile from its GetColorSpace. Recovery should + // fail, because it has been pushed out of the cache. + ICCProfile recovered_1; + EXPECT_FALSE(original_color_space_0.GetICCProfile(&recovered_1)); + EXPECT_NE(original, recovered_1); + + // Create an identical ICCProfile to the original. It should equal the + // original, because the comparison is based on data. + ICCProfile identical_1 = ICCProfileForTestingNoAnalyticTrFn(); + EXPECT_EQ(original, identical_1); + + // The identical ICCProfile's GetColorSpace will not match, because the + // original points to the now-uncached version. + ColorSpace identical_1_color_space = identical_1.GetColorSpace(); + EXPECT_NE(identical_1_color_space, original_color_space_0); + + // The original ICCProfile is now orphaned because there exists a new entry + // with the same data. + ColorSpace original_color_space_2 = original.GetColorSpace(); + ICCProfile recovered_2; + EXPECT_FALSE(original_color_space_2.GetICCProfile(&recovered_2)); + EXPECT_NE(original, recovered_2); + + // Blow away the cache one more time. + for (size_t i = 0; i < 128; ++i) { + SkMatrix44 toXYZD50; + ColorSpace::CreateSRGB().GetPrimaryMatrix(&toXYZD50); + SkColorSpaceTransferFn fn; + fn.fA = 1; + fn.fB = 0; + fn.fC = 1; + fn.fD = 0; + fn.fE = 0; + fn.fF = 0; + fn.fG = 1.5f + i / 128.f; + ColorSpace color_space = ColorSpace::CreateCustom(toXYZD50, fn); + ICCProfile icc_profile; + color_space.GetICCProfile(&icc_profile); + } + + // Now the original ICCProfile can re-insert itself into the cache, with its + // original id. + ColorSpace original_color_space_3 = original.GetColorSpace(); + ICCProfile recovered_3; + EXPECT_TRUE(original_color_space_3.GetICCProfile(&recovered_3)); + EXPECT_EQ(original, recovered_3); +} + } // namespace gfx diff --git a/chromium/ui/gfx/image/canvas_image_source.h b/chromium/ui/gfx/image/canvas_image_source.h index 1108f0d7c6d..1b1846635c9 100644 --- a/chromium/ui/gfx/image/canvas_image_source.h +++ b/chromium/ui/gfx/image/canvas_image_source.h @@ -5,6 +5,8 @@ #ifndef UI_GFX_IMAGE_CANVAS_IMAGE_SOURCE_H_ #define UI_GFX_IMAGE_CANVAS_IMAGE_SOURCE_H_ +#include <utility> + #include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/ptr_util.h" @@ -30,7 +32,7 @@ class GFX_EXPORT CanvasImageSource : public gfx::ImageSkiaSource { static ImageSkia MakeImageSkia(Args&&... args) { auto source = base::MakeUnique<T>(std::forward<Args>(args)...); gfx::Size size = source->size(); - return gfx::ImageSkia(source.release(), size); + return gfx::ImageSkia(std::move(source), size); } CanvasImageSource(const gfx::Size& size, bool is_opaque); diff --git a/chromium/ui/gfx/image/image.cc b/chromium/ui/gfx/image/image.cc index b2984fb0f3d..7b49776f9ed 100644 --- a/chromium/ui/gfx/image/image.cc +++ b/chromium/ui/gfx/image/image.cc @@ -5,8 +5,8 @@ #include "ui/gfx/image/image.h" #include <algorithm> -#include <set> #include <utility> +#include <vector> #include "base/logging.h" #include "base/macros.h" @@ -14,13 +14,9 @@ #include "build/build_config.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/geometry/size.h" +#include "ui/gfx/image/image_platform.h" #include "ui/gfx/image/image_png_rep.h" #include "ui/gfx/image/image_skia.h" -#include "ui/gfx/image/image_skia_source.h" - -#if !defined(OS_IOS) -#include "ui/gfx/codec/png_codec.h" -#endif #if defined(OS_IOS) #include "base/mac/foundation_util.h" @@ -33,136 +29,14 @@ namespace gfx { -namespace internal { - -#if defined(OS_IOS) -scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromUIImage( - UIImage* uiimage); -// Caller takes ownership of the returned UIImage. -UIImage* CreateUIImageFromPNG( - const std::vector<ImagePNGRep>& image_png_reps); -gfx::Size UIImageSize(UIImage* image); -#elif defined(OS_MACOSX) -scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromNSImage( - NSImage* nsimage); -// Caller takes ownership of the returned NSImage. -NSImage* NSImageFromPNG(const std::vector<ImagePNGRep>& image_png_reps, - CGColorSpaceRef color_space); -gfx::Size NSImageSize(NSImage* image); -#endif // defined(OS_MACOSX) - -#if defined(OS_IOS) -ImageSkia* ImageSkiaFromPNG( - const std::vector<ImagePNGRep>& image_png_reps); -scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromImageSkia( - const ImageSkia* skia); -#else -// Returns a 16x16 red image to visually show error in decoding PNG. -// Caller takes ownership of returned ImageSkia. -ImageSkia* GetErrorImageSkia() { - SkBitmap bitmap; - bitmap.allocN32Pixels(16, 16); - bitmap.eraseARGB(0xff, 0xff, 0, 0); - return new ImageSkia(ImageSkiaRep(bitmap, 1.0f)); -} - -class PNGImageSource : public ImageSkiaSource { - public: - PNGImageSource() {} - ~PNGImageSource() override {} - - ImageSkiaRep GetImageForScale(float scale) override { - if (image_skia_reps_.empty()) - return ImageSkiaRep(); - - const ImageSkiaRep* rep = NULL; - // gfx::ImageSkia passes one of the resource scale factors. The source - // should return: - // 1) The ImageSkiaRep with the highest scale if all available - // scales are smaller than |scale|. - // 2) The ImageSkiaRep with the smallest one that is larger than |scale|. - for (ImageSkiaRepSet::const_iterator iter = image_skia_reps_.begin(); - iter != image_skia_reps_.end(); ++iter) { - if ((*iter).scale() == scale) - return (*iter); - if (!rep || rep->scale() < (*iter).scale()) - rep = &(*iter); - if (rep->scale() >= scale) - break; - } - return rep ? *rep : ImageSkiaRep(); - } - - const gfx::Size size() const { - return size_; - } - - bool AddPNGData(const ImagePNGRep& png_rep) { - const gfx::ImageSkiaRep rep = ToImageSkiaRep(png_rep); - if (rep.is_null()) - return false; - if (size_.IsEmpty()) - size_ = gfx::Size(rep.GetWidth(), rep.GetHeight()); - image_skia_reps_.insert(rep); - return true; - } +namespace { - static ImageSkiaRep ToImageSkiaRep(const ImagePNGRep& png_rep) { - scoped_refptr<base::RefCountedMemory> raw_data = png_rep.raw_data; - CHECK(raw_data.get()); - SkBitmap bitmap; - if (!PNGCodec::Decode(raw_data->front(), raw_data->size(), - &bitmap)) { - LOG(ERROR) << "Unable to decode PNG for " << png_rep.scale << "."; - return ImageSkiaRep(); - } - return ImageSkiaRep(bitmap, png_rep.scale); - } +using RepresentationMap = + std::map<Image::RepresentationType, std::unique_ptr<internal::ImageRep>>; - private: - struct Compare { - bool operator()(const ImageSkiaRep& rep1, const ImageSkiaRep& rep2) { - return rep1.scale() < rep2.scale(); - } - }; +} // namespace - typedef std::set<ImageSkiaRep, Compare> ImageSkiaRepSet; - ImageSkiaRepSet image_skia_reps_; - gfx::Size size_; - - DISALLOW_COPY_AND_ASSIGN(PNGImageSource); -}; - -ImageSkia* ImageSkiaFromPNG( - const std::vector<ImagePNGRep>& image_png_reps) { - if (image_png_reps.empty()) - return GetErrorImageSkia(); - std::unique_ptr<PNGImageSource> image_source(new PNGImageSource); - - for (size_t i = 0; i < image_png_reps.size(); ++i) { - if (!image_source->AddPNGData(image_png_reps[i])) - return GetErrorImageSkia(); - } - const gfx::Size& size = image_source->size(); - DCHECK(!size.IsEmpty()); - if (size.IsEmpty()) - return GetErrorImageSkia(); - return new ImageSkia(image_source.release(), size); -} - -scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromImageSkia( - const ImageSkia* image_skia) { - ImageSkiaRep image_skia_rep = image_skia->GetRepresentation(1.0f); - - scoped_refptr<base::RefCountedBytes> png_bytes(new base::RefCountedBytes()); - if (image_skia_rep.scale() != 1.0f || - !PNGCodec::EncodeBGRASkBitmap(image_skia_rep.sk_bitmap(), false, - &png_bytes->data())) { - return NULL; - } - return png_bytes; -} -#endif +namespace internal { class ImageRepPNG; class ImageRepSkia; @@ -182,25 +56,41 @@ class ImageRep { virtual ~ImageRep() {} // Cast helpers ("fake RTTI"). - ImageRepPNG* AsImageRepPNG() { + const ImageRepPNG* AsImageRepPNG() const { CHECK_EQ(type_, Image::kImageRepPNG); - return reinterpret_cast<ImageRepPNG*>(this); + return reinterpret_cast<const ImageRepPNG*>(this); + } + ImageRepPNG* AsImageRepPNG() { + return const_cast<ImageRepPNG*>( + static_cast<const ImageRep*>(this)->AsImageRepPNG()); } - ImageRepSkia* AsImageRepSkia() { + const ImageRepSkia* AsImageRepSkia() const { CHECK_EQ(type_, Image::kImageRepSkia); - return reinterpret_cast<ImageRepSkia*>(this); + return reinterpret_cast<const ImageRepSkia*>(this); + } + ImageRepSkia* AsImageRepSkia() { + return const_cast<ImageRepSkia*>( + static_cast<const ImageRep*>(this)->AsImageRepSkia()); } #if defined(OS_IOS) - ImageRepCocoaTouch* AsImageRepCocoaTouch() { + const ImageRepCocoaTouch* AsImageRepCocoaTouch() const { CHECK_EQ(type_, Image::kImageRepCocoaTouch); - return reinterpret_cast<ImageRepCocoaTouch*>(this); + return reinterpret_cast<const ImageRepCocoaTouch*>(this); + } + ImageRepCocoaTouch* AsImageRepCocoaTouch() { + return const_cast<ImageRepCocoaTouch*>( + static_cast<const ImageRep*>(this)->AsImageRepCocoaTouch()); } #elif defined(OS_MACOSX) - ImageRepCocoa* AsImageRepCocoa() { + const ImageRepCocoa* AsImageRepCocoa() const { CHECK_EQ(type_, Image::kImageRepCocoa); - return reinterpret_cast<ImageRepCocoa*>(this); + return reinterpret_cast<const ImageRepCocoa*>(this); + } + ImageRepCocoa* AsImageRepCocoa() { + return const_cast<ImageRepCocoa*>( + static_cast<const ImageRep*>(this)->AsImageRepCocoa()); } #endif @@ -273,6 +163,7 @@ class ImageRepSkia : public ImageRep { gfx::Size Size() const override { return image_->size(); } + const ImageSkia* image() const { return image_.get(); } ImageSkia* image() { return image_.get(); } private: @@ -356,16 +247,51 @@ class ImageStorage : public base::RefCounted<ImageStorage> { { } - Image::RepresentationType default_representation_type() { + Image::RepresentationType default_representation_type() const { + DCHECK(IsOnValidSequence()); return default_representation_type_; } - Image::RepresentationMap& representations() { return representations_; } + + bool HasRepresentation(Image::RepresentationType type) const { + DCHECK(IsOnValidSequence()); + return representations_.count(type) != 0; + } + + size_t RepresentationCount() const { + DCHECK(IsOnValidSequence()); + return representations_.size(); + } + + const ImageRep* GetRepresentation(Image::RepresentationType rep_type, + bool must_exist) const { + DCHECK(IsOnValidSequence()); + RepresentationMap::const_iterator it = representations_.find(rep_type); + if (it == representations_.end()) { + CHECK(!must_exist); + return NULL; + } + return it->second.get(); + } + + const ImageRep* AddRepresentation(std::unique_ptr<ImageRep> rep) const { + DCHECK(IsOnValidSequence()); + Image::RepresentationType type = rep->type(); + auto result = representations_.insert(std::make_pair(type, std::move(rep))); + + // insert should not fail (implies that there was already a representation + // of that type in the map). + CHECK(result.second) << "type was already in map."; + + return result.first->second.get(); + } #if defined(OS_MACOSX) && !defined(OS_IOS) void set_default_representation_color_space(CGColorSpaceRef color_space) { + DCHECK(IsOnValidSequence()); default_representation_color_space_ = color_space; } - CGColorSpaceRef default_representation_color_space() { + CGColorSpaceRef default_representation_color_space() const { + DCHECK(IsOnValidSequence()); return default_representation_color_space_; } #endif // defined(OS_MACOSX) && !defined(OS_IOS) @@ -389,7 +315,7 @@ class ImageStorage : public base::RefCounted<ImageStorage> { // All the representations of an Image. Size will always be at least one, with // more for any converted representations. - Image::RepresentationMap representations_; + mutable RepresentationMap representations_; DISALLOW_COPY_AND_ASSIGN(ImageStorage); }; @@ -443,16 +369,15 @@ Image::Image(NSImage* image) { } #endif -Image::Image(const Image& other) : storage_(other.storage_) { -} +Image::Image(const Image& other) = default; -Image& Image::operator=(const Image& other) { - storage_ = other.storage_; - return *this; -} +Image::Image(Image&& other) = default; -Image::~Image() { -} +Image& Image::operator=(const Image& other) = default; + +Image& Image::operator=(Image&& other) = default; + +Image::~Image() {} // static Image Image::CreateFrom1xBitmap(const SkBitmap& bitmap) { @@ -487,12 +412,12 @@ const SkBitmap* Image::ToSkBitmap() const { } const ImageSkia* Image::ToImageSkia() const { - internal::ImageRep* rep = GetRepresentation(kImageRepSkia, false); + const internal::ImageRep* rep = GetRepresentation(kImageRepSkia, false); if (!rep) { std::unique_ptr<internal::ImageRep> scoped_rep; switch (DefaultRepresentationType()) { case kImageRepPNG: { - internal::ImageRepPNG* png_rep = + const internal::ImageRepPNG* png_rep = GetRepresentation(kImageRepPNG, true)->AsImageRepPNG(); scoped_rep.reset(new internal::ImageRepSkia( internal::ImageSkiaFromPNG(png_rep->image_reps()))); @@ -500,7 +425,7 @@ const ImageSkia* Image::ToImageSkia() const { } #if defined(OS_IOS) case kImageRepCocoaTouch: { - internal::ImageRepCocoaTouch* native_rep = + const internal::ImageRepCocoaTouch* native_rep = GetRepresentation(kImageRepCocoaTouch, true) ->AsImageRepCocoaTouch(); scoped_rep.reset(new internal::ImageRepSkia( @@ -509,7 +434,7 @@ const ImageSkia* Image::ToImageSkia() const { } #elif defined(OS_MACOSX) case kImageRepCocoa: { - internal::ImageRepCocoa* native_rep = + const internal::ImageRepCocoa* native_rep = GetRepresentation(kImageRepCocoa, true)->AsImageRepCocoa(); scoped_rep.reset(new internal::ImageRepSkia( new ImageSkia(ImageSkiaFromNSImage(native_rep->image())))); @@ -527,19 +452,19 @@ const ImageSkia* Image::ToImageSkia() const { #if defined(OS_IOS) UIImage* Image::ToUIImage() const { - internal::ImageRep* rep = GetRepresentation(kImageRepCocoaTouch, false); + const internal::ImageRep* rep = GetRepresentation(kImageRepCocoaTouch, false); if (!rep) { std::unique_ptr<internal::ImageRep> scoped_rep; switch (DefaultRepresentationType()) { case kImageRepPNG: { - internal::ImageRepPNG* png_rep = + const internal::ImageRepPNG* png_rep = GetRepresentation(kImageRepPNG, true)->AsImageRepPNG(); scoped_rep.reset(new internal::ImageRepCocoaTouch( internal::CreateUIImageFromPNG(png_rep->image_reps()))); break; } case kImageRepSkia: { - internal::ImageRepSkia* skia_rep = + const internal::ImageRepSkia* skia_rep = GetRepresentation(kImageRepSkia, true)->AsImageRepSkia(); UIImage* image = UIImageFromImageSkia(*skia_rep->image()); base::mac::NSObjectRetain(image); @@ -556,22 +481,22 @@ UIImage* Image::ToUIImage() const { } #elif defined(OS_MACOSX) NSImage* Image::ToNSImage() const { - internal::ImageRep* rep = GetRepresentation(kImageRepCocoa, false); + const internal::ImageRep* rep = GetRepresentation(kImageRepCocoa, false); if (!rep) { std::unique_ptr<internal::ImageRep> scoped_rep; CGColorSpaceRef default_representation_color_space = - storage_->default_representation_color_space(); + storage()->default_representation_color_space(); switch (DefaultRepresentationType()) { case kImageRepPNG: { - internal::ImageRepPNG* png_rep = + const internal::ImageRepPNG* png_rep = GetRepresentation(kImageRepPNG, true)->AsImageRepPNG(); scoped_rep.reset(new internal::ImageRepCocoa(internal::NSImageFromPNG( png_rep->image_reps(), default_representation_color_space))); break; } case kImageRepSkia: { - internal::ImageRepSkia* skia_rep = + const internal::ImageRepSkia* skia_rep = GetRepresentation(kImageRepSkia, true)->AsImageRepSkia(); NSImage* image = NSImageFromImageSkiaWithColorSpace(*skia_rep->image(), default_representation_color_space); @@ -593,7 +518,7 @@ scoped_refptr<base::RefCountedMemory> Image::As1xPNGBytes() const { if (IsEmpty()) return new base::RefCountedBytes(); - internal::ImageRep* rep = GetRepresentation(kImageRepPNG, false); + const internal::ImageRep* rep = GetRepresentation(kImageRepPNG, false); if (rep) { const std::vector<ImagePNGRep>& image_png_reps = @@ -609,23 +534,22 @@ scoped_refptr<base::RefCountedMemory> Image::As1xPNGBytes() const { switch (DefaultRepresentationType()) { #if defined(OS_IOS) case kImageRepCocoaTouch: { - internal::ImageRepCocoaTouch* cocoa_touch_rep = - GetRepresentation(kImageRepCocoaTouch, true) - ->AsImageRepCocoaTouch(); + const internal::ImageRepCocoaTouch* cocoa_touch_rep = + GetRepresentation(kImageRepCocoaTouch, true)->AsImageRepCocoaTouch(); png_bytes = internal::Get1xPNGBytesFromUIImage( cocoa_touch_rep->image()); break; } #elif defined(OS_MACOSX) case kImageRepCocoa: { - internal::ImageRepCocoa* cocoa_rep = + const internal::ImageRepCocoa* cocoa_rep = GetRepresentation(kImageRepCocoa, true)->AsImageRepCocoa(); png_bytes = internal::Get1xPNGBytesFromNSImage(cocoa_rep->image()); break; } #endif case kImageRepSkia: { - internal::ImageRepSkia* skia_rep = + const internal::ImageRepSkia* skia_rep = GetRepresentation(kImageRepSkia, true)->AsImageRepSkia(); png_bytes = internal::Get1xPNGBytesFromImageSkia(skia_rep->image()); break; @@ -697,14 +621,11 @@ NSImage* Image::CopyNSImage() const { #endif bool Image::HasRepresentation(RepresentationType type) const { - return storage_.get() && storage_->representations().count(type) != 0; + return storage() && storage()->HasRepresentation(type); } size_t Image::RepresentationCount() const { - if (!storage_.get()) - return 0; - - return storage_->representations().size(); + return storage() ? storage()->RepresentationCount() : 0; } bool Image::IsEmpty() const { @@ -729,46 +650,28 @@ gfx::Size Image::Size() const { return GetRepresentation(DefaultRepresentationType(), true)->Size(); } -void Image::SwapRepresentations(Image* other) { - storage_.swap(other->storage_); -} - #if defined(OS_MACOSX) && !defined(OS_IOS) void Image::SetSourceColorSpace(CGColorSpaceRef color_space) { - if (storage_.get()) - storage_->set_default_representation_color_space(color_space); + if (storage()) + storage()->set_default_representation_color_space(color_space); } #endif // defined(OS_MACOSX) && !defined(OS_IOS) Image::RepresentationType Image::DefaultRepresentationType() const { - CHECK(storage_.get()); - return storage_->default_representation_type(); + CHECK(storage()); + return storage()->default_representation_type(); } -internal::ImageRep* Image::GetRepresentation( - RepresentationType rep_type, bool must_exist) const { - CHECK(storage_.get()); - RepresentationMap::const_iterator it = - storage_->representations().find(rep_type); - if (it == storage_->representations().end()) { - CHECK(!must_exist); - return NULL; - } - return it->second.get(); +const internal::ImageRep* Image::GetRepresentation(RepresentationType rep_type, + bool must_exist) const { + CHECK(storage()); + return storage()->GetRepresentation(rep_type, must_exist); } -internal::ImageRep* Image::AddRepresentation( +const internal::ImageRep* Image::AddRepresentation( std::unique_ptr<internal::ImageRep> rep) const { - CHECK(storage_.get()); - RepresentationType type = rep->type(); - auto result = - storage_->representations().insert(std::make_pair(type, std::move(rep))); - - // insert should not fail (implies that there was already a representation of - // that type in the map). - CHECK(result.second) << "type was already in map."; - - return result.first->second.get(); + CHECK(storage()); + return storage()->AddRepresentation(std::move(rep)); } } // namespace gfx diff --git a/chromium/ui/gfx/image/image.h b/chromium/ui/gfx/image/image.h index 01344242d93..f0d6cbb0b2f 100644 --- a/chromium/ui/gfx/image/image.h +++ b/chromium/ui/gfx/image/image.h @@ -56,9 +56,6 @@ class GFX_EXPORT Image { kImageRepPNG, }; - using RepresentationMap = - std::map<RepresentationType, std::unique_ptr<internal::ImageRep>>; - // Creates an empty image with no representations. Image(); @@ -87,9 +84,17 @@ class GFX_EXPORT Image { // Initializes a new Image by AddRef()ing |other|'s internal storage. Image(const Image& other); + // Moves a reference from |other| to the new image without changing the + // reference count. + Image(Image&& other); + // Copies a reference to |other|'s storage. Image& operator=(const Image& other); + // Moves a reference from |other|'s storage without changing the reference + // count. + Image& operator=(Image&& other); + // Deletes the image and, if the only owner of the storage, all of its cached // representations. ~Image(); @@ -169,9 +174,6 @@ class GFX_EXPORT Image { int Height() const; gfx::Size Size() const; - // Swaps this image's internal representations with |other|. - void SwapRepresentations(gfx::Image* other); - #if defined(OS_MACOSX) && !defined(OS_IOS) // Set the default representation's color space. This is used for converting // to NSImage. This is used to compensate for PNGCodec not writing or reading @@ -185,15 +187,21 @@ class GFX_EXPORT Image { // Returns the ImageRep of the appropriate type or NULL if there is no // representation of that type (and must_exist is false). - internal::ImageRep* GetRepresentation( - RepresentationType rep_type, bool must_exist) const; + const internal::ImageRep* GetRepresentation(RepresentationType rep_type, + bool must_exist) const; // Stores a representation into the map. A representation of that type must // not already be in the map. Returns a pointer to the representation stored // inside the map. - internal::ImageRep* AddRepresentation( + const internal::ImageRep* AddRepresentation( std::unique_ptr<internal::ImageRep> rep) const; + // Getter should be used internally (unless a handle to the scoped_refptr is + // needed) instead of directly accessing |storage_|, to ensure logical + // constness is upheld. + const internal::ImageStorage* storage() const { return storage_.get(); } + internal::ImageStorage* storage() { return storage_.get(); } + // Internal class that holds all the representations. This allows the Image to // be cheaply copied. scoped_refptr<internal::ImageStorage> storage_; diff --git a/chromium/ui/gfx/image/image_family.cc b/chromium/ui/gfx/image/image_family.cc index b9d28bddd9d..0adc6692336 100644 --- a/chromium/ui/gfx/image/image_family.cc +++ b/chromium/ui/gfx/image/image_family.cc @@ -25,8 +25,17 @@ ImageFamily::const_iterator::const_iterator( ImageFamily::const_iterator::~const_iterator() {} ImageFamily::ImageFamily() {} +ImageFamily::ImageFamily(ImageFamily&& other) = default; ImageFamily::~ImageFamily() {} +ImageFamily& ImageFamily::operator=(ImageFamily&& other) = default; + +ImageFamily ImageFamily::Clone() const { + ImageFamily clone; + clone.map_ = map_; + return clone; +} + void ImageFamily::Add(const gfx::Image& image) { gfx::Size size = image.Size(); if (size.IsEmpty()) { diff --git a/chromium/ui/gfx/image/image_family.h b/chromium/ui/gfx/image/image_family.h index e8920b9a88f..3d0890a425d 100644 --- a/chromium/ui/gfx/image/image_family.h +++ b/chromium/ui/gfx/image/image_family.h @@ -98,8 +98,11 @@ class GFX_EXPORT ImageFamily { }; ImageFamily(); + ImageFamily(ImageFamily&& other); ~ImageFamily(); + ImageFamily& operator=(ImageFamily&& other); + // Gets an iterator to the first image. const_iterator begin() const { return const_iterator(map_.begin()); } // Gets an iterator to one after the last image. @@ -111,6 +114,10 @@ class GFX_EXPORT ImageFamily { // Removes all images from the family. void clear() { return map_.clear(); } + // Creates a shallow copy of the family. The Images inside share their backing + // store with the original Images. + ImageFamily Clone() const; + // Adds an image to the family. If another image is already present at the // same size, it will be overwritten. void Add(const gfx::Image& image); @@ -162,6 +169,12 @@ class GFX_EXPORT ImageFamily { // Map from (aspect ratio, width) to image. std::map<MapKey, gfx::Image> map_; + + // Even though the Images in the family are copyable (reference-counted), the + // family itself should not be implicitly copied, as it would result in a + // shallow clone of the entire map and updates to many reference counts. + // ImageFamily can be explicitly Clone()d, but std::move is preferred. + DISALLOW_COPY_AND_ASSIGN(ImageFamily); }; } // namespace gfx diff --git a/chromium/ui/gfx/image/image_family_unittest.cc b/chromium/ui/gfx/image/image_family_unittest.cc index 88d52981400..e3973d037ac 100644 --- a/chromium/ui/gfx/image/image_family_unittest.cc +++ b/chromium/ui/gfx/image/image_family_unittest.cc @@ -69,6 +69,26 @@ TEST_F(ImageFamilyTest, Clear) { EXPECT_TRUE(image_family_.empty()); } +TEST_F(ImageFamilyTest, MoveConstructor) { + gfx::ImageFamily family(std::move(image_family_)); + EXPECT_TRUE(image_family_.empty()); + EXPECT_FALSE(family.empty()); +} + +TEST_F(ImageFamilyTest, MoveAssignment) { + gfx::ImageFamily family; + EXPECT_TRUE(family.empty()); + family = std::move(image_family_); + EXPECT_TRUE(image_family_.empty()); + EXPECT_FALSE(family.empty()); +} + +TEST_F(ImageFamilyTest, Clone) { + gfx::ImageFamily family = image_family_.Clone(); + EXPECT_FALSE(image_family_.empty()); + EXPECT_FALSE(family.empty()); +} + // Tests iteration over an ImageFamily. TEST_F(ImageFamilyTest, Iteration) { gfx::ImageFamily::const_iterator it = image_family_.begin(); diff --git a/chromium/ui/gfx/image/image_generic.cc b/chromium/ui/gfx/image/image_generic.cc new file mode 100644 index 00000000000..3f424915dc9 --- /dev/null +++ b/chromium/ui/gfx/image/image_generic.cc @@ -0,0 +1,123 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/gfx/image/image_platform.h" + +#include <set> +#include <utility> + +#include "ui/gfx/codec/png_codec.h" +#include "ui/gfx/image/image_skia_source.h" + +namespace gfx { +namespace internal { + +namespace { + +// Returns a 16x16 red image to visually show error in decoding PNG. +// Caller takes ownership of returned ImageSkia. +ImageSkia* GetErrorImageSkia() { + SkBitmap bitmap; + bitmap.allocN32Pixels(16, 16); + bitmap.eraseARGB(0xff, 0xff, 0, 0); + return new ImageSkia(ImageSkiaRep(bitmap, 1.0f)); +} + +class PNGImageSource : public ImageSkiaSource { + public: + PNGImageSource() {} + ~PNGImageSource() override {} + + ImageSkiaRep GetImageForScale(float scale) override { + if (image_skia_reps_.empty()) + return ImageSkiaRep(); + + const ImageSkiaRep* rep = NULL; + // gfx::ImageSkia passes one of the resource scale factors. The source + // should return: + // 1) The ImageSkiaRep with the highest scale if all available + // scales are smaller than |scale|. + // 2) The ImageSkiaRep with the smallest one that is larger than |scale|. + for (ImageSkiaRepSet::const_iterator iter = image_skia_reps_.begin(); + iter != image_skia_reps_.end(); ++iter) { + if ((*iter).scale() == scale) + return (*iter); + if (!rep || rep->scale() < (*iter).scale()) + rep = &(*iter); + if (rep->scale() >= scale) + break; + } + return rep ? *rep : ImageSkiaRep(); + } + + const gfx::Size size() const { return size_; } + + bool AddPNGData(const ImagePNGRep& png_rep) { + const gfx::ImageSkiaRep rep = ToImageSkiaRep(png_rep); + if (rep.is_null()) + return false; + if (size_.IsEmpty()) + size_ = gfx::Size(rep.GetWidth(), rep.GetHeight()); + image_skia_reps_.insert(rep); + return true; + } + + static ImageSkiaRep ToImageSkiaRep(const ImagePNGRep& png_rep) { + scoped_refptr<base::RefCountedMemory> raw_data = png_rep.raw_data; + CHECK(raw_data.get()); + SkBitmap bitmap; + if (!PNGCodec::Decode(raw_data->front(), raw_data->size(), &bitmap)) { + LOG(ERROR) << "Unable to decode PNG for " << png_rep.scale << "."; + return ImageSkiaRep(); + } + return ImageSkiaRep(bitmap, png_rep.scale); + } + + private: + struct Compare { + bool operator()(const ImageSkiaRep& rep1, const ImageSkiaRep& rep2) { + return rep1.scale() < rep2.scale(); + } + }; + + typedef std::set<ImageSkiaRep, Compare> ImageSkiaRepSet; + ImageSkiaRepSet image_skia_reps_; + gfx::Size size_; + + DISALLOW_COPY_AND_ASSIGN(PNGImageSource); +}; + +} // namespace + +ImageSkia* ImageSkiaFromPNG(const std::vector<ImagePNGRep>& image_png_reps) { + if (image_png_reps.empty()) + return GetErrorImageSkia(); + std::unique_ptr<PNGImageSource> image_source(new PNGImageSource); + + for (size_t i = 0; i < image_png_reps.size(); ++i) { + if (!image_source->AddPNGData(image_png_reps[i])) + return GetErrorImageSkia(); + } + const gfx::Size& size = image_source->size(); + DCHECK(!size.IsEmpty()); + if (size.IsEmpty()) + return GetErrorImageSkia(); + return new ImageSkia(std::move(image_source), size); +} + +scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromImageSkia( + const ImageSkia* image_skia) { + ImageSkiaRep image_skia_rep = image_skia->GetRepresentation(1.0f); + + scoped_refptr<base::RefCountedBytes> png_bytes(new base::RefCountedBytes()); + if (image_skia_rep.scale() != 1.0f || + !PNGCodec::EncodeBGRASkBitmap(image_skia_rep.sk_bitmap(), false, + &png_bytes->data())) { + return NULL; + } + return png_bytes; +} + +} // namespace internal +} // namespace gfx diff --git a/chromium/ui/gfx/image/image_ios.mm b/chromium/ui/gfx/image/image_ios.mm index 9520f32082b..e1c644d1b5f 100644 --- a/chromium/ui/gfx/image/image_ios.mm +++ b/chromium/ui/gfx/image/image_ios.mm @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "ui/gfx/image/image.h" +#include "ui/gfx/image/image_platform.h" #include <stddef.h> #import <UIKit/UIKit.h> diff --git a/chromium/ui/gfx/image/image_mac.mm b/chromium/ui/gfx/image/image_mac.mm index 6358a36dc2d..bce798620b3 100644 --- a/chromium/ui/gfx/image/image_mac.mm +++ b/chromium/ui/gfx/image/image_mac.mm @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "ui/gfx/image/image.h" +#include "ui/gfx/image/image_platform.h" #import <AppKit/AppKit.h> #include <stddef.h> diff --git a/chromium/ui/gfx/image/image_platform.h b/chromium/ui/gfx/image/image_platform.h new file mode 100644 index 00000000000..ca49cbebb13 --- /dev/null +++ b/chromium/ui/gfx/image/image_platform.h @@ -0,0 +1,57 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// This file declares platform-specific helper functions in gfx::internal for +// use by image.cc. +// +// The functions are implemented in image_generic.cc (all platforms other than +// iOS), image_ios.mm and image_mac.mm. + +#ifndef UI_GFX_IMAGE_IMAGE_PLATFORM_H_ +#define UI_GFX_IMAGE_IMAGE_PLATFORM_H_ + +#include <vector> + +#include "base/memory/ref_counted.h" +#include "base/memory/ref_counted_memory.h" +#include "build/build_config.h" +#include "ui/gfx/geometry/size.h" +#include "ui/gfx/image/image_png_rep.h" +#include "ui/gfx/image/image_skia.h" + +#if defined(OS_IOS) +#include "base/mac/foundation_util.h" +#include "ui/gfx/image/image_skia_util_ios.h" +#elif defined(OS_MACOSX) +#include "base/mac/foundation_util.h" +#include "base/mac/mac_util.h" +#include "ui/gfx/image/image_skia_util_mac.h" +#endif + +namespace gfx { +namespace internal { + +#if defined(OS_IOS) +scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromUIImage( + UIImage* uiimage); +// Caller takes ownership of the returned UIImage. +UIImage* CreateUIImageFromPNG(const std::vector<ImagePNGRep>& image_png_reps); +gfx::Size UIImageSize(UIImage* image); +#elif defined(OS_MACOSX) +scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromNSImage( + NSImage* nsimage); +// Caller takes ownership of the returned NSImage. +NSImage* NSImageFromPNG(const std::vector<ImagePNGRep>& image_png_reps, + CGColorSpaceRef color_space); +gfx::Size NSImageSize(NSImage* image); +#endif // defined(OS_MACOSX) + +ImageSkia* ImageSkiaFromPNG(const std::vector<ImagePNGRep>& image_png_reps); +scoped_refptr<base::RefCountedMemory> Get1xPNGBytesFromImageSkia( + const ImageSkia* image_skia); + +} // namespace internal +} // namespace gfx + +#endif // UI_GFX_IMAGE_IMAGE_PLATFORM_H_ diff --git a/chromium/ui/gfx/image/image_skia.cc b/chromium/ui/gfx/image/image_skia.cc index bfcd9bb04a8..1ceeaf9916e 100644 --- a/chromium/ui/gfx/image/image_skia.cc +++ b/chromium/ui/gfx/image/image_skia.cc @@ -299,9 +299,6 @@ ImageSkia::ImageSkia(std::unique_ptr<ImageSkiaSource> source, float scale) DetachStorageFromSequence(); } -ImageSkia::ImageSkia(ImageSkiaSource* source, const gfx::Size& size) - : ImageSkia(base::WrapUnique(source), size) {} - ImageSkia::ImageSkia(const ImageSkiaRep& image_rep) { Init(image_rep); // No other thread has reference to this, so it's safe to detach the sequence. diff --git a/chromium/ui/gfx/image/image_skia.h b/chromium/ui/gfx/image/image_skia.h index 02f1ae21bcf..aa04e584336 100644 --- a/chromium/ui/gfx/image/image_skia.h +++ b/chromium/ui/gfx/image/image_skia.h @@ -53,10 +53,6 @@ class GFX_EXPORT ImageSkia { // at |scale| and uses its dimensions to calculate the size in DIP. ImageSkia(std::unique_ptr<ImageSkiaSource> source, float scale); - // Deprecated versions of the above constructors. ImageSkia takes ownership of - // |source|. - ImageSkia(ImageSkiaSource* source, const gfx::Size& size); - explicit ImageSkia(const gfx::ImageSkiaRep& image_rep); // Copies a reference to |other|'s storage. diff --git a/chromium/ui/gfx/image/image_skia_operations.cc b/chromium/ui/gfx/image/image_skia_operations.cc index f48d2e1b9c9..df5186a8a79 100644 --- a/chromium/ui/gfx/image/image_skia_operations.cc +++ b/chromium/ui/gfx/image/image_skia_operations.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/logging.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "skia/ext/image_operations.h" #include "third_party/skia/include/core/SkClipOp.h" #include "third_party/skia/include/core/SkDrawLooper.h" @@ -489,7 +490,8 @@ ImageSkia ImageSkiaOperations::CreateBlendedImage(const ImageSkia& first, if (first.isNull() || second.isNull()) return ImageSkia(); - return ImageSkia(new BlendingImageSource(first, second, alpha), first.size()); + return ImageSkia(base::MakeUnique<BlendingImageSource>(first, second, alpha), + first.size()); } // static @@ -499,7 +501,8 @@ ImageSkia ImageSkiaOperations::CreateSuperimposedImage( if (first.isNull() || second.isNull()) return ImageSkia(); - return ImageSkia(new SuperimposedImageSource(first, second), first.size()); + return ImageSkia(base::MakeUnique<SuperimposedImageSource>(first, second), + first.size()); } // static @@ -508,7 +511,8 @@ ImageSkia ImageSkiaOperations::CreateTransparentImage(const ImageSkia& image, if (image.isNull()) return ImageSkia(); - return ImageSkia(new TransparentImageSource(image, alpha), image.size()); + return ImageSkia(base::MakeUnique<TransparentImageSource>(image, alpha), + image.size()); } // static @@ -517,7 +521,7 @@ ImageSkia ImageSkiaOperations::CreateMaskedImage(const ImageSkia& rgb, if (rgb.isNull() || alpha.isNull()) return ImageSkia(); - return ImageSkia(new MaskedImageSource(rgb, alpha), rgb.size()); + return ImageSkia(base::MakeUnique<MaskedImageSource>(rgb, alpha), rgb.size()); } // static @@ -527,8 +531,9 @@ ImageSkia ImageSkiaOperations::CreateTiledImage(const ImageSkia& source, if (source.isNull()) return ImageSkia(); - return ImageSkia(new TiledImageSource(source, src_x, src_y, dst_w, dst_h), - gfx::Size(dst_w, dst_h)); + return ImageSkia( + base::MakeUnique<TiledImageSource>(source, src_x, src_y, dst_w, dst_h), + gfx::Size(dst_w, dst_h)); } // static @@ -538,7 +543,8 @@ ImageSkia ImageSkiaOperations::CreateHSLShiftedImage( if (image.isNull()) return ImageSkia(); - return ImageSkia(new HSLImageSource(image, hsl_shift), image.size()); + return ImageSkia(base::MakeUnique<HSLImageSource>(image, hsl_shift), + image.size()); } // static @@ -548,7 +554,8 @@ ImageSkia ImageSkiaOperations::CreateButtonBackground(SkColor color, if (image.isNull() || mask.isNull()) return ImageSkia(); - return ImageSkia(new ButtonImageSource(color, image, mask), mask.size()); + return ImageSkia(base::MakeUnique<ButtonImageSource>(color, image, mask), + mask.size()); } // static @@ -560,8 +567,9 @@ ImageSkia ImageSkiaOperations::ExtractSubset(const ImageSkia& image, return ImageSkia(); } - return ImageSkia(new ExtractSubsetImageSource(image, clipped_bounds), - clipped_bounds.size()); + return ImageSkia( + base::MakeUnique<ExtractSubsetImageSource>(image, clipped_bounds), + clipped_bounds.size()); } // static @@ -572,8 +580,9 @@ ImageSkia ImageSkiaOperations::CreateResizedImage( if (source.isNull()) return ImageSkia(); - return ImageSkia(new ResizeSource(source, method, target_dip_size), - target_dip_size); + return ImageSkia( + base::MakeUnique<ResizeSource>(source, method, target_dip_size), + target_dip_size); } // static @@ -587,7 +596,8 @@ ImageSkia ImageSkiaOperations::CreateImageWithDropShadow( gfx::Size shadow_image_size = source.size(); shadow_image_size.Enlarge(shadow_padding.width(), shadow_padding.height()); - return ImageSkia(new DropShadowSource(source, shadows), shadow_image_size); + return ImageSkia(base::MakeUnique<DropShadowSource>(source, shadows), + shadow_image_size); } // static @@ -595,7 +605,7 @@ ImageSkia ImageSkiaOperations::CreateHorizontalShadow( const std::vector<ShadowValue>& shadows, bool fades_down) { auto* source = new HorizontalShadowSource(shadows, fades_down); - return ImageSkia(source, source->size()); + return ImageSkia(base::WrapUnique(source), source->size()); } // static @@ -605,11 +615,10 @@ ImageSkia ImageSkiaOperations::CreateRotatedImage( if (source.isNull()) return ImageSkia(); - return ImageSkia(new RotatedSource(source, rotation), - SkBitmapOperations::ROTATION_180_CW == rotation ? - source.size() : - gfx::Size(source.height(), source.width())); - + return ImageSkia(base::MakeUnique<RotatedSource>(source, rotation), + SkBitmapOperations::ROTATION_180_CW == rotation + ? source.size() + : gfx::Size(source.height(), source.width())); } // static @@ -621,7 +630,8 @@ ImageSkia ImageSkiaOperations::CreateIconWithBadge(const ImageSkia& icon, if (badge.isNull()) return icon; - return ImageSkia(new IconWithBadgeSource(icon, badge), icon.size()); + return ImageSkia(base::MakeUnique<IconWithBadgeSource>(icon, badge), + icon.size()); } } // namespace gfx diff --git a/chromium/ui/gfx/image/image_skia_unittest.cc b/chromium/ui/gfx/image/image_skia_unittest.cc index df9b42dbed3..971325e1ad7 100644 --- a/chromium/ui/gfx/image/image_skia_unittest.cc +++ b/chromium/ui/gfx/image/image_skia_unittest.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/logging.h" #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "base/threading/simple_thread.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -154,7 +155,7 @@ class ImageSkiaTest : public testing::Test { TEST_F(ImageSkiaTest, FixedSource) { ImageSkiaRep image(Size(100, 200), 0.0f); - ImageSkia image_skia(new FixedSource(image), Size(100, 200)); + ImageSkia image_skia(base::MakeUnique<FixedSource>(image), Size(100, 200)); EXPECT_EQ(0U, image_skia.image_reps().size()); const ImageSkiaRep& result_100p = image_skia.GetRepresentation(1.0f); @@ -191,7 +192,8 @@ TEST_F(ImageSkiaTest, FixedSource) { TEST_F(ImageSkiaTest, FixedScaledSource) { ImageSkiaRep image(Size(100, 200), 1.0f); - ImageSkia image_skia(new FixedScaleSource(image), Size(100, 200)); + ImageSkia image_skia(base::MakeUnique<FixedScaleSource>(image), + Size(100, 200)); EXPECT_EQ(0U, image_skia.image_reps().size()); const ImageSkiaRep& result_100p = image_skia.GetRepresentation(1.0f); @@ -219,7 +221,8 @@ TEST_F(ImageSkiaTest, FixedScaledSource) { TEST_F(ImageSkiaTest, FixedUnscaledSource) { ImageSkiaRep image(Size(100, 200), 0.0f); - ImageSkia image_skia(new FixedScaleSource(image), Size(100, 200)); + ImageSkia image_skia(base::MakeUnique<FixedScaleSource>(image), + Size(100, 200)); EXPECT_EQ(0U, image_skia.image_reps().size()); const ImageSkiaRep& result_100p = image_skia.GetRepresentation(1.0f); @@ -244,7 +247,8 @@ TEST_F(ImageSkiaTest, FixedUnscaledSource) { } TEST_F(ImageSkiaTest, DynamicSource) { - ImageSkia image_skia(new DynamicSource(Size(100, 200)), Size(100, 200)); + ImageSkia image_skia(base::MakeUnique<DynamicSource>(Size(100, 200)), + Size(100, 200)); EXPECT_EQ(0U, image_skia.image_reps().size()); const ImageSkiaRep& result_100p = image_skia.GetRepresentation(1.0f); EXPECT_EQ(100, result_100p.GetWidth()); @@ -277,7 +281,8 @@ TEST_F(ImageSkiaTest, ManyRepsPerScaleFactor) { const int kSmallIcon2x = 32; const int kLargeIcon1x = 32; - ImageSkia image(new NullSource(), gfx::Size(kSmallIcon1x, kSmallIcon1x)); + ImageSkia image(base::MakeUnique<NullSource>(), + gfx::Size(kSmallIcon1x, kSmallIcon1x)); // Simulate a source which loads images on a delay. Upon // GetImageForScaleFactor, it immediately returns null and starts loading // image reps slowly. @@ -308,7 +313,8 @@ TEST_F(ImageSkiaTest, ManyRepsPerScaleFactor) { } TEST_F(ImageSkiaTest, GetBitmap) { - ImageSkia image_skia(new DynamicSource(Size(100, 200)), Size(100, 200)); + ImageSkia image_skia(base::MakeUnique<DynamicSource>(Size(100, 200)), + Size(100, 200)); const SkBitmap* bitmap = image_skia.bitmap(); EXPECT_NE(static_cast<SkBitmap*>(NULL), bitmap); EXPECT_FALSE(bitmap->isNull()); @@ -427,7 +433,8 @@ TEST_F(ImageSkiaTest, StaticOnThreadTest) { } TEST_F(ImageSkiaTest, SourceOnThreadTest) { - ImageSkia image(new DynamicSource(Size(100, 200)), Size(100, 200)); + ImageSkia image(base::MakeUnique<DynamicSource>(Size(100, 200)), + Size(100, 200)); EXPECT_FALSE(image.IsThreadSafe()); test::TestOnThread image_on_thread(&image); @@ -507,7 +514,7 @@ std::vector<float> GetSortedScaleFactors(const gfx::ImageSkia& image) { TEST_F(ImageSkiaTest, ArbitraryScaleFactor) { // source is owned by |image| DynamicSource* source = new DynamicSource(Size(100, 200)); - ImageSkia image(source, gfx::Size(100, 200)); + ImageSkia image(base::WrapUnique(source), gfx::Size(100, 200)); image.GetRepresentation(1.5f); EXPECT_EQ(2.0f, source->GetLastRequestedScaleAndReset()); @@ -579,8 +586,9 @@ TEST_F(ImageSkiaTest, ArbitraryScaleFactor) { } TEST_F(ImageSkiaTest, ArbitraryScaleFactorWithMissingResource) { - ImageSkia image(new FixedScaleSource( - ImageSkiaRep(Size(100, 200), 1.0f)), Size(100, 200)); + ImageSkia image( + base::MakeUnique<FixedScaleSource>(ImageSkiaRep(Size(100, 200), 1.0f)), + Size(100, 200)); // Requesting 1.5f -- falls back to 2.0f, but couldn't find. It should // look up 1.0f and then rescale it. Note that the rescaled ImageSkiaRep will @@ -594,8 +602,9 @@ TEST_F(ImageSkiaTest, ArbitraryScaleFactorWithMissingResource) { TEST_F(ImageSkiaTest, UnscaledImageForArbitraryScaleFactor) { // 0.0f means unscaled. - ImageSkia image(new FixedScaleSource( - ImageSkiaRep(Size(100, 200), 0.0f)), Size(100, 200)); + ImageSkia image( + base::MakeUnique<FixedScaleSource>(ImageSkiaRep(Size(100, 200), 0.0f)), + Size(100, 200)); // Requesting 2.0f, which should return 1.0f unscaled image. const ImageSkiaRep& rep = image.GetRepresentation(2.0f); diff --git a/chromium/ui/gfx/image/image_unittest.cc b/chromium/ui/gfx/image/image_unittest.cc index 317e59830fd..1e0ccd3ac93 100644 --- a/chromium/ui/gfx/image/image_unittest.cc +++ b/chromium/ui/gfx/image/image_unittest.cc @@ -50,26 +50,6 @@ TEST_F(ImageTest, EmptyImage) { EXPECT_TRUE(image.IsEmpty()); EXPECT_EQ(0, image.Width()); EXPECT_EQ(0, image.Height()); - - // Test the copy constructor. - gfx::Image imageCopy(image); - EXPECT_TRUE(imageCopy.IsEmpty()); - EXPECT_EQ(0, imageCopy.Width()); - EXPECT_EQ(0, imageCopy.Height()); - - // Test calling SwapRepresentations() with an empty image. - gfx::Image image2(gt::CreateImageSkia(25, 25)); - EXPECT_FALSE(image2.IsEmpty()); - EXPECT_EQ(25, image2.Width()); - EXPECT_EQ(25, image2.Height()); - - image.SwapRepresentations(&image2); - EXPECT_FALSE(image.IsEmpty()); - EXPECT_EQ(25, image.Width()); - EXPECT_EQ(25, image.Height()); - EXPECT_TRUE(image2.IsEmpty()); - EXPECT_EQ(0, image2.Width()); - EXPECT_EQ(0, image2.Height()); } // Test constructing a gfx::Image from an empty PlatformImage. @@ -590,61 +570,72 @@ TEST_F(ImageTest, SkBitmapConversionPreservesTransparency) { } } -TEST_F(ImageTest, SwapRepresentations) { +TEST_F(ImageTest, Copy) { const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U; gfx::Image image1(gt::CreateImageSkia(25, 25)); - const gfx::ImageSkia* image_skia1 = image1.ToImageSkia(); + EXPECT_EQ(25, image1.Width()); + EXPECT_EQ(25, image1.Height()); + gfx::Image image2(image1); + EXPECT_EQ(25, image2.Width()); + EXPECT_EQ(25, image2.Height()); + EXPECT_EQ(1U, image1.RepresentationCount()); + EXPECT_EQ(1U, image2.RepresentationCount()); + EXPECT_EQ(image1.ToImageSkia(), image2.ToImageSkia()); - gfx::Image image2(gt::CreatePlatformImage()); - const gfx::ImageSkia* image_skia2 = image2.ToImageSkia(); - gt::PlatformImage platform_image = gt::ToPlatformType(image2); + EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image2))); EXPECT_EQ(kRepCount, image2.RepresentationCount()); + EXPECT_EQ(kRepCount, image1.RepresentationCount()); +} - image1.SwapRepresentations(&image2); +TEST_F(ImageTest, Assign) { + gfx::Image image1(gt::CreatePlatformImage()); + EXPECT_EQ(25, image1.Width()); + EXPECT_EQ(25, image1.Height()); + // Assignment must be on a separate line to the declaration in order to test + // assignment operator (instead of copy constructor). + gfx::Image image2; + image2 = image1; + EXPECT_EQ(25, image2.Width()); + EXPECT_EQ(25, image2.Height()); - EXPECT_EQ(image_skia2, image1.ToImageSkia()); - EXPECT_TRUE(gt::PlatformImagesEqual(platform_image, - gt::ToPlatformType(image1))); - EXPECT_EQ(image_skia1, image2.ToImageSkia()); - EXPECT_EQ(kRepCount, image1.RepresentationCount()); + EXPECT_EQ(1U, image1.RepresentationCount()); EXPECT_EQ(1U, image2.RepresentationCount()); + EXPECT_EQ(image1.ToSkBitmap(), image2.ToSkBitmap()); } -TEST_F(ImageTest, Copy) { +TEST_F(ImageTest, Move) { const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U; gfx::Image image1(gt::CreateImageSkia(25, 25)); EXPECT_EQ(25, image1.Width()); EXPECT_EQ(25, image1.Height()); - gfx::Image image2(image1); + gfx::Image image2(std::move(image1)); EXPECT_EQ(25, image2.Width()); EXPECT_EQ(25, image2.Height()); - EXPECT_EQ(1U, image1.RepresentationCount()); + EXPECT_EQ(0U, image1.RepresentationCount()); EXPECT_EQ(1U, image2.RepresentationCount()); - EXPECT_EQ(image1.ToImageSkia(), image2.ToImageSkia()); EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image2))); + EXPECT_EQ(0U, image1.RepresentationCount()); EXPECT_EQ(kRepCount, image2.RepresentationCount()); - EXPECT_EQ(kRepCount, image1.RepresentationCount()); } -TEST_F(ImageTest, Assign) { +TEST_F(ImageTest, MoveAssign) { gfx::Image image1(gt::CreatePlatformImage()); EXPECT_EQ(25, image1.Width()); EXPECT_EQ(25, image1.Height()); // Assignment must be on a separate line to the declaration in order to test - // assignment operator (instead of copy constructor). + // move assignment operator (instead of move constructor). gfx::Image image2; - image2 = image1; + image2 = std::move(image1); EXPECT_EQ(25, image2.Width()); EXPECT_EQ(25, image2.Height()); - EXPECT_EQ(1U, image1.RepresentationCount()); + EXPECT_EQ(0U, image1.RepresentationCount()); EXPECT_EQ(1U, image2.RepresentationCount()); - EXPECT_EQ(image1.ToSkBitmap(), image2.ToSkBitmap()); } TEST_F(ImageTest, MultiResolutionImageSkia) { diff --git a/chromium/ui/gfx/image/mojo/image_skia_struct_traits.cc b/chromium/ui/gfx/image/mojo/image_skia_struct_traits.cc index 98ef823b972..cb9487fee78 100644 --- a/chromium/ui/gfx/image/mojo/image_skia_struct_traits.cc +++ b/chromium/ui/gfx/image/mojo/image_skia_struct_traits.cc @@ -107,32 +107,12 @@ bool StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep>::Read( } // static -void* StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::SetUpContext( +std::vector<gfx::ImageSkiaRep> +StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::image_reps( const gfx::ImageSkia& input) { // Trigger the image to load everything. input.EnsureRepsForSupportedScales(); - - // Use a context to return a stable list of ImageSkiaRep objects. That is, - // multiple calls of image_reps() should return exactly the same list of - // ImageSkiaRep objects. So that ImageSkiaRep with the same backing pixel - // buffer is properly serialized and only once. - return new std::vector<gfx::ImageSkiaRep>(input.image_reps()); -} - -// static -void StructTraits<gfx::mojom::ImageSkiaDataView, - gfx::ImageSkia>::TearDownContext(const gfx::ImageSkia& input, - void* context) { - delete static_cast<std::vector<gfx::ImageSkiaRep>*>(context); -} - -// static -const std::vector<gfx::ImageSkiaRep>& -StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia>::image_reps( - const gfx::ImageSkia& input, - void* context) { - // See the comment in SetUpContext regarding context usage. - return *(static_cast<std::vector<gfx::ImageSkiaRep>*>(context)); + return input.image_reps(); } // static diff --git a/chromium/ui/gfx/image/mojo/image_skia_struct_traits.h b/chromium/ui/gfx/image/mojo/image_skia_struct_traits.h index 974ec9cf1d7..401b282789e 100644 --- a/chromium/ui/gfx/image/mojo/image_skia_struct_traits.h +++ b/chromium/ui/gfx/image/mojo/image_skia_struct_traits.h @@ -56,15 +56,9 @@ struct StructTraits<gfx::mojom::ImageSkiaRepDataView, gfx::ImageSkiaRep> { template <> struct StructTraits<gfx::mojom::ImageSkiaDataView, gfx::ImageSkia> { - static void* SetUpContext(const gfx::ImageSkia& input); - static void TearDownContext(const gfx::ImageSkia& input, void* context); - static const std::vector<gfx::ImageSkiaRep>& image_reps( - const gfx::ImageSkia& input, - void* context); + static std::vector<gfx::ImageSkiaRep> image_reps(const gfx::ImageSkia& input); - static bool IsNull(const gfx::ImageSkia& input) { - return input.image_reps().empty(); - } + static bool IsNull(const gfx::ImageSkia& input) { return input.isNull(); } static void SetToNull(gfx::ImageSkia* out) { *out = gfx::ImageSkia(); } static bool Read(gfx::mojom::ImageSkiaDataView data, gfx::ImageSkia* out); diff --git a/chromium/ui/gfx/image/mojo/image_traits_unittest.cc b/chromium/ui/gfx/image/mojo/image_traits_unittest.cc index 6f35c10289e..086790a35cf 100644 --- a/chromium/ui/gfx/image/mojo/image_traits_unittest.cc +++ b/chromium/ui/gfx/image/mojo/image_traits_unittest.cc @@ -6,6 +6,7 @@ #include <vector> #include "base/macros.h" +#include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" #include "mojo/public/cpp/bindings/binding_set.h" #include "testing/gtest/include/gtest/gtest.h" @@ -130,20 +131,22 @@ TEST_F(ImageTraitsTest, NullImageSkia) { EXPECT_TRUE(output.isNull()); } -TEST_F(ImageTraitsTest, ImageSkiaWithNoRepsTreatedAsNull) { +TEST_F(ImageTraitsTest, ImageSkiaRepsAreCreatedAsNeeded) { const gfx::Size kSize(1, 2); - ImageSkia image(new TestImageSkiaSource(kSize), kSize); - ASSERT_FALSE(image.isNull()); + ImageSkia image(base::MakeUnique<TestImageSkiaSource>(kSize), kSize); + EXPECT_FALSE(image.isNull()); + EXPECT_TRUE(image.image_reps().empty()); - ImageSkia output(ImageSkiaRep(gfx::Size(1, 1), 1.0f)); - ASSERT_FALSE(output.isNull()); - service()->EchoImageSkia(image, &output); + ImageSkia output; EXPECT_TRUE(output.isNull()); + service()->EchoImageSkia(image, &output); + EXPECT_FALSE(image.image_reps().empty()); + EXPECT_FALSE(output.isNull()); } TEST_F(ImageTraitsTest, ImageSkia) { const gfx::Size kSize(1, 2); - ImageSkia image(new TestImageSkiaSource(kSize), kSize); + ImageSkia image(base::MakeUnique<TestImageSkiaSource>(kSize), kSize); image.GetRepresentation(1.0f); image.GetRepresentation(2.0f); @@ -155,7 +158,7 @@ TEST_F(ImageTraitsTest, ImageSkia) { TEST_F(ImageTraitsTest, EmptyRepPreserved) { const gfx::Size kSize(1, 2); - ImageSkia image(new TestImageSkiaSource(kSize), kSize); + ImageSkia image(base::MakeUnique<TestImageSkiaSource>(kSize), kSize); image.GetRepresentation(1.0f); SkBitmap empty_bitmap; @@ -170,7 +173,7 @@ TEST_F(ImageTraitsTest, EmptyRepPreserved) { TEST_F(ImageTraitsTest, ImageSkiaWithOperations) { const gfx::Size kSize(32, 32); - ImageSkia image(new TestImageSkiaSource(kSize), kSize); + ImageSkia image(base::MakeUnique<TestImageSkiaSource>(kSize), kSize); const gfx::Size kNewSize(16, 16); ImageSkia resized = ImageSkiaOperations::CreateResizedImage( diff --git a/chromium/ui/gfx/linux/client_native_pixmap_factory_dmabuf.cc b/chromium/ui/gfx/linux/client_native_pixmap_factory_dmabuf.cc index a91e50645fe..e5d11bb830b 100644 --- a/chromium/ui/gfx/linux/client_native_pixmap_factory_dmabuf.cc +++ b/chromium/ui/gfx/linux/client_native_pixmap_factory_dmabuf.cc @@ -68,6 +68,8 @@ class ClientNativePixmapFactoryDmabuf : public ClientNativePixmapFactory { format == gfx::BufferFormat::BGRA_8888 || format == gfx::BufferFormat::RGBX_8888 || format == gfx::BufferFormat::RGBA_8888; + case gfx::BufferUsage::SCANOUT_VDA_WRITE: + return false; case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE: case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE_PERSISTENT: { #if defined(OS_CHROMEOS) @@ -83,6 +85,18 @@ class ClientNativePixmapFactoryDmabuf : public ClientNativePixmapFactory { return false; #endif } + case gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE: { +#if defined(OS_CHROMEOS) + // Each platform only supports one camera buffer type. We list the + // supported buffer formats on all platforms here. When allocating a + // camera buffer the caller is responsible for making sure a buffer is + // successfully allocated. For example, allocating YUV420_BIPLANAR + // for SCANOUT_CAMERA_READ_WRITE may only work on Intel boards. + return format == gfx::BufferFormat::YUV_420_BIPLANAR; +#else + return false; +#endif + } } NOTREACHED(); return false; @@ -96,6 +110,7 @@ class ClientNativePixmapFactoryDmabuf : public ClientNativePixmapFactory { case gfx::BufferUsage::SCANOUT_CPU_READ_WRITE: case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE: case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE_PERSISTENT: + case gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE: #if defined(OS_CHROMEOS) return ClientNativePixmapDmaBuf::ImportFromDmabuf(handle, size); #else @@ -104,6 +119,7 @@ class ClientNativePixmapFactoryDmabuf : public ClientNativePixmapFactory { #endif case gfx::BufferUsage::GPU_READ: case gfx::BufferUsage::SCANOUT: + case gfx::BufferUsage::SCANOUT_VDA_WRITE: // Close all the fds. for (const auto& fd : handle.fds) base::ScopedFD scoped_fd(fd.fd); diff --git a/chromium/ui/gfx/mojo/BUILD.gn b/chromium/ui/gfx/mojo/BUILD.gn index 9722968cbd9..b71ee2532d3 100644 --- a/chromium/ui/gfx/mojo/BUILD.gn +++ b/chromium/ui/gfx/mojo/BUILD.gn @@ -29,15 +29,3 @@ mojom("test_interfaces") { ":mojo", ] } - -source_set("struct_traits") { - sources = [ - "selection_bound_struct_traits.h", - "transform_struct_traits.h", - ] - public_deps = [ - ":mojo_shared_cpp_sources", - "//mojo/public/cpp/bindings", - "//ui/gfx", - ] -} diff --git a/chromium/ui/gfx/mojo/buffer_types.mojom b/chromium/ui/gfx/mojo/buffer_types.mojom index 29c50d54179..79b22417a02 100644 --- a/chromium/ui/gfx/mojo/buffer_types.mojom +++ b/chromium/ui/gfx/mojo/buffer_types.mojom @@ -32,7 +32,9 @@ enum BufferFormat { enum BufferUsage { GPU_READ, SCANOUT, + SCANOUT_CAMERA_READ_WRITE, SCANOUT_CPU_READ_WRITE, + SCANOUT_VDA_WRITE, GPU_READ_CPU_READ_WRITE, GPU_READ_CPU_READ_WRITE_PERSISTENT, diff --git a/chromium/ui/gfx/mojo/buffer_types_struct_traits.cc b/chromium/ui/gfx/mojo/buffer_types_struct_traits.cc index 186e2eb5502..e9aea16a63c 100644 --- a/chromium/ui/gfx/mojo/buffer_types_struct_traits.cc +++ b/chromium/ui/gfx/mojo/buffer_types_struct_traits.cc @@ -8,30 +8,17 @@ namespace mojo { -void* StructTraits<gfx::mojom::NativePixmapHandleDataView, - gfx::NativePixmapHandle>:: - SetUpContext(const gfx::NativePixmapHandle& pixmap_handle) { - auto* handles = new std::vector<mojo::ScopedHandle>(); +std::vector<mojo::ScopedHandle> +StructTraits<gfx::mojom::NativePixmapHandleDataView, gfx::NativePixmapHandle>:: + fds(const gfx::NativePixmapHandle& pixmap_handle) { + std::vector<mojo::ScopedHandle> handles; #if defined(OS_LINUX) for (const base::FileDescriptor& fd : pixmap_handle.fds) - handles->emplace_back(mojo::WrapPlatformFile(fd.fd)); + handles.emplace_back(mojo::WrapPlatformFile(fd.fd)); #endif // defined(OS_LINUX) return handles; } -void StructTraits<gfx::mojom::NativePixmapHandleDataView, - gfx::NativePixmapHandle>:: - TearDownContext(const gfx::NativePixmapHandle& handle, void* context) { - delete static_cast<std::vector<mojo::ScopedHandle>*>(context); -} - -std::vector<mojo::ScopedHandle>& StructTraits< - gfx::mojom::NativePixmapHandleDataView, - gfx::NativePixmapHandle>::fds(const gfx::NativePixmapHandle& pixmap_handle, - void* context) { - return *static_cast<std::vector<mojo::ScopedHandle>*>(context); -} - bool StructTraits< gfx::mojom::NativePixmapHandleDataView, gfx::NativePixmapHandle>::Read(gfx::mojom::NativePixmapHandleDataView data, diff --git a/chromium/ui/gfx/mojo/buffer_types_struct_traits.h b/chromium/ui/gfx/mojo/buffer_types_struct_traits.h index 0fdf909f5fc..368d499c43d 100644 --- a/chromium/ui/gfx/mojo/buffer_types_struct_traits.h +++ b/chromium/ui/gfx/mojo/buffer_types_struct_traits.h @@ -126,8 +126,12 @@ struct EnumTraits<gfx::mojom::BufferUsage, gfx::BufferUsage> { return gfx::mojom::BufferUsage::GPU_READ; case gfx::BufferUsage::SCANOUT: return gfx::mojom::BufferUsage::SCANOUT; + case gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE: + return gfx::mojom::BufferUsage::SCANOUT_CAMERA_READ_WRITE; case gfx::BufferUsage::SCANOUT_CPU_READ_WRITE: return gfx::mojom::BufferUsage::SCANOUT_CPU_READ_WRITE; + case gfx::BufferUsage::SCANOUT_VDA_WRITE: + return gfx::mojom::BufferUsage::SCANOUT_VDA_WRITE; case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE: return gfx::mojom::BufferUsage::GPU_READ_CPU_READ_WRITE; case gfx::BufferUsage::GPU_READ_CPU_READ_WRITE_PERSISTENT: @@ -145,9 +149,15 @@ struct EnumTraits<gfx::mojom::BufferUsage, gfx::BufferUsage> { case gfx::mojom::BufferUsage::SCANOUT: *out = gfx::BufferUsage::SCANOUT; return true; + case gfx::mojom::BufferUsage::SCANOUT_CAMERA_READ_WRITE: + *out = gfx::BufferUsage::SCANOUT_CAMERA_READ_WRITE; + return true; case gfx::mojom::BufferUsage::SCANOUT_CPU_READ_WRITE: *out = gfx::BufferUsage::SCANOUT_CPU_READ_WRITE; return true; + case gfx::mojom::BufferUsage::SCANOUT_VDA_WRITE: + *out = gfx::BufferUsage::SCANOUT_VDA_WRITE; + return true; case gfx::mojom::BufferUsage::GPU_READ_CPU_READ_WRITE: *out = gfx::BufferUsage::GPU_READ_CPU_READ_WRITE; return true; @@ -239,10 +249,6 @@ struct StructTraits<gfx::mojom::NativePixmapPlaneDataView, template <> struct StructTraits<gfx::mojom::NativePixmapHandleDataView, gfx::NativePixmapHandle> { - static void* SetUpContext(const gfx::NativePixmapHandle& handle); - static void TearDownContext(const gfx::NativePixmapHandle& handle, - void* context); - static bool IsNull(const gfx::NativePixmapHandle& handle) { #if defined(OS_LINUX) return false; @@ -251,9 +257,8 @@ struct StructTraits<gfx::mojom::NativePixmapHandleDataView, return true; #endif } - static std::vector<mojo::ScopedHandle>& fds( - const gfx::NativePixmapHandle& pixmap_handle, - void* context); + static std::vector<mojo::ScopedHandle> fds( + const gfx::NativePixmapHandle& pixmap_handle); static const std::vector<gfx::NativePixmapPlane>& planes( const gfx::NativePixmapHandle& pixmap_handle) { diff --git a/chromium/ui/gfx/mojo/selection_bound.typemap b/chromium/ui/gfx/mojo/selection_bound.typemap index d188a3a234a..e2b16f7a264 100644 --- a/chromium/ui/gfx/mojo/selection_bound.typemap +++ b/chromium/ui/gfx/mojo/selection_bound.typemap @@ -5,7 +5,4 @@ mojom = "//ui/gfx/mojo/selection_bound.mojom" public_headers = [ "//ui/gfx/selection_bound.h" ] traits_headers = [ "//ui/gfx/mojo/selection_bound_struct_traits.h" ] -deps = [ - "//ui/gfx/mojo:struct_traits", -] type_mappings = [ "gfx.mojom.SelectionBound=gfx::SelectionBound" ] diff --git a/chromium/ui/gfx/mojo/transform.typemap b/chromium/ui/gfx/mojo/transform.typemap index 28da3090487..b675c8f1df4 100644 --- a/chromium/ui/gfx/mojo/transform.typemap +++ b/chromium/ui/gfx/mojo/transform.typemap @@ -5,7 +5,4 @@ mojom = "//ui/gfx/mojo/transform.mojom" public_headers = [ "//ui/gfx/transform.h" ] traits_headers = [ "//ui/gfx/mojo/transform_struct_traits.h" ] -deps = [ - "//ui/gfx/mojo:struct_traits", -] type_mappings = [ "gfx.mojom.Transform=gfx::Transform" ] diff --git a/chromium/ui/gfx/native_widget_types.h b/chromium/ui/gfx/native_widget_types.h index fbe0fef01ac..17f9be2b46e 100644 --- a/chromium/ui/gfx/native_widget_types.h +++ b/chromium/ui/gfx/native_widget_types.h @@ -11,7 +11,7 @@ #include "build/build_config.h" #if defined(OS_ANDROID) -#include <jni.h> +#include "base/android/scoped_java_ref.h" #elif defined(OS_MACOSX) #include <objc/objc.h> #endif @@ -104,7 +104,7 @@ class ViewAndroid; #endif class SkBitmap; -#if defined(USE_X11) && !defined(OS_CHROMEOS) +#if defined(USE_X11) extern "C" { struct _AtkObject; typedef struct _AtkObject AtkObject; @@ -132,13 +132,7 @@ typedef NSEvent* NativeEvent; typedef void* NativeCursor; typedef ui::ViewAndroid* NativeView; typedef ui::WindowAndroid* NativeWindow; -typedef jobject NativeEvent; -#elif defined(OS_FUCHSIA) -// TODO(fuchsia): Update when we have a plan for UI on Fuchsia. -typedef void* NativeCursor; -typedef void* NativeView; -typedef void* NativeWindow; -typedef void* NativeEvent; +typedef base::android::ScopedJavaGlobalRef<jobject> NativeEvent; #else #error Unknown build environment. #endif @@ -154,7 +148,7 @@ typedef NSFont* NativeFont; typedef id NativeViewAccessible; #else // Android, Linux, Chrome OS, etc. // Linux doesn't have a native font type. -#if defined(USE_X11) && !defined(OS_CHROMEOS) +#if defined(USE_X11) typedef AtkObject* NativeViewAccessible; #else typedef void* NativeViewAccessible; @@ -203,10 +197,6 @@ constexpr AcceleratedWidget kNullAcceleratedWidget = 0; #elif defined(USE_OZONE) typedef int32_t AcceleratedWidget; constexpr AcceleratedWidget kNullAcceleratedWidget = 0; -#elif defined(OS_FUCHSIA) -// TODO(fuchsia): Update when we have a plan for UI on Fuchsia. -typedef void* AcceleratedWidget; -constexpr AcceleratedWidget kNullAcceleratedWidget = 0; #else #error unknown platform #endif diff --git a/chromium/ui/gfx/paint_vector_icon.cc b/chromium/ui/gfx/paint_vector_icon.cc index 0c642bf4ec6..97edb15adeb 100644 --- a/chromium/ui/gfx/paint_vector_icon.cc +++ b/chromium/ui/gfx/paint_vector_icon.cc @@ -66,6 +66,7 @@ class PathParser { case V_LINE_TO: case R_V_LINE_TO: case CANVAS_DIMENSIONS: + case PATH_COLOR_ALPHA: return 1; case MOVE_TO: @@ -123,6 +124,7 @@ CommandType CommandFromString(const std::string& source) { return command; RETURN_IF_IS(NEW_PATH); + RETURN_IF_IS(PATH_COLOR_ALPHA); RETURN_IF_IS(PATH_COLOR_ARGB); RETURN_IF_IS(PATH_MODE_CLEAR); RETURN_IF_IS(STROKE); @@ -213,6 +215,10 @@ void PaintPath(Canvas* canvas, case NEW_PATH: break; + case PATH_COLOR_ALPHA: + flags.setAlpha(SkScalarFloorToInt(arg(0))); + break; + case PATH_COLOR_ARGB: flags.setColor(SkColorSetARGB( SkScalarFloorToInt(arg(0)), SkScalarFloorToInt(arg(1)), @@ -392,8 +398,9 @@ void PaintPath(Canvas* canvas, if (elapsed_time >= delay + duration) { state = 1; } else if (elapsed_time > delay) { - state = (elapsed_time - delay).ToInternalValue() / - static_cast<double>(duration.ToInternalValue()); + DCHECK(!duration.is_zero()); + state = (elapsed_time - delay).InMicroseconds() / + static_cast<double>(duration.InMicroseconds()); } auto weight = Tween::CalculateValue( @@ -432,13 +439,15 @@ void PaintPath(Canvas* canvas, previous_command_type = command_type; } - ScopedRTLFlipCanvas scoped_rtl_flip_canvas(canvas, canvas_size, flips_in_rtl); + ScopedCanvas scoped_canvas(canvas); if (dip_size != canvas_size) { SkScalar scale = SkIntToScalar(dip_size) / SkIntToScalar(canvas_size); canvas->sk_canvas()->scale(scale, scale); } + ScopedRTLFlipCanvas scoped_rtl_flip_canvas(canvas, canvas_size, flips_in_rtl); + if (!clip_rect.isEmpty()) canvas->sk_canvas()->clipRect(clip_rect); @@ -497,7 +506,7 @@ class VectorIconCache { if (iter != images_.end()) return iter->second; - ImageSkia icon_image(new VectorIconSource(description), + ImageSkia icon_image(base::MakeUnique<VectorIconSource>(description), Size(description.dip_size, description.dip_size)); images_.insert(std::make_pair(description, icon_image)); return icon_image; diff --git a/chromium/ui/gfx/platform_font_fuchsia.cc b/chromium/ui/gfx/platform_font_fuchsia.cc new file mode 100644 index 00000000000..d1500668323 --- /dev/null +++ b/chromium/ui/gfx/platform_font_fuchsia.cc @@ -0,0 +1,26 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "ui/gfx/platform_font.h" + +#include "base/logging.h" + +namespace gfx { + +// static +PlatformFont* PlatformFont::CreateDefault() { + // TODO(fuchsia): Stubbed during headless bringup, https://crbug.com/743296. + NOTIMPLEMENTED(); + return NULL; +} + +// static +PlatformFont* PlatformFont::CreateFromNameAndSize(const std::string& font_name, + int font_size) { + // TODO(fuchsia): Stubbed during headless bringup, https://crbug.com/743296. + NOTIMPLEMENTED(); + return NULL; +} + +} // namespace gfx diff --git a/chromium/ui/gfx/platform_font_win.cc b/chromium/ui/gfx/platform_font_win.cc index 100ee944050..35f5a212bcd 100644 --- a/chromium/ui/gfx/platform_font_win.cc +++ b/chromium/ui/gfx/platform_font_win.cc @@ -395,6 +395,11 @@ void PlatformFontWin::SetDirectWriteFactory(IDWriteFactory* factory) { } // static +bool PlatformFontWin::IsDirectWriteEnabled() { + return direct_write_factory_ != nullptr; +} + +// static void PlatformFontWin::GetTextMetricsForFont(HDC hdc, HFONT font, TEXTMETRIC* text_metrics) { @@ -465,7 +470,7 @@ PlatformFontWin::HFontRef* PlatformFontWin::CreateHFontRef(HFONT font) { GetTextMetricsForFont(screen_dc, font, &font_metrics); } - if (direct_write_factory_) + if (IsDirectWriteEnabled()) return CreateHFontRefFromSkia(font, font_metrics); return CreateHFontRefFromGDI(font, font_metrics); diff --git a/chromium/ui/gfx/platform_font_win.h b/chromium/ui/gfx/platform_font_win.h index ac4f6640802..5e8e72f4283 100644 --- a/chromium/ui/gfx/platform_font_win.h +++ b/chromium/ui/gfx/platform_font_win.h @@ -71,6 +71,8 @@ class GFX_EXPORT PlatformFontWin : public PlatformFont { // from skia and DirectWrite. static void SetDirectWriteFactory(IDWriteFactory* factory); + static bool IsDirectWriteEnabled(); + // Returns the GDI metrics for the font passed in. static void GetTextMetricsForFont(HDC hdc, HFONT font, diff --git a/chromium/ui/gfx/render_text.cc b/chromium/ui/gfx/render_text.cc index 041c9f27ee4..f8999eb9f61 100644 --- a/chromium/ui/gfx/render_text.cc +++ b/chromium/ui/gfx/render_text.cc @@ -18,6 +18,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/trace_event/trace_event.h" #include "build/build_config.h" +#include "cc/base/math_util.h" #include "cc/paint/paint_canvas.h" #include "cc/paint/paint_shader.h" #include "third_party/icu/source/common/unicode/rbbi.h" @@ -231,8 +232,16 @@ void SkiaTextRenderer::SetShader(sk_sp<cc::PaintShader> shader) { void SkiaTextRenderer::DrawPosText(const SkPoint* pos, const uint16_t* glyphs, size_t glyph_count) { - const size_t byte_length = glyph_count * sizeof(glyphs[0]); - canvas_skia_->drawPosText(&glyphs[0], byte_length, &pos[0], flags_); + SkTextBlobBuilder builder; + const auto& run_buffer = builder.allocRunPos(flags_.ToSkPaint(), glyph_count); + + static_assert(sizeof(*glyphs) == sizeof(*run_buffer.glyphs), ""); + memcpy(run_buffer.glyphs, glyphs, glyph_count * sizeof(*glyphs)); + + static_assert(sizeof(*pos) == 2 * sizeof(*run_buffer.pos), ""); + memcpy(run_buffer.pos, pos, glyph_count * sizeof(*pos)); + + canvas_skia_->drawTextBlob(builder.make(), 0, 0, flags_); } void SkiaTextRenderer::DrawUnderline(int x, int y, int width) { @@ -1424,11 +1433,34 @@ base::string16 RenderText::Elide(const base::string16& text, StringSlicer slicer(text, ellipsis, elide_in_middle, elide_at_beginning); - // Use binary search to compute the elided text. + // Use binary(-like) search to compute the elided text. In particular, do + // an interpolation search, which is a binary search in which each guess + // is an attempt to smartly calculate the right point rather than blindly + // guessing midway between the endpoints. size_t lo = 0; size_t hi = text.length() - 1; + size_t guess = std::string::npos; + // These two widths are not exactly right but they're good enough to provide + // some guidance to the search. For example, |text_width| is actually the + // length of text.length(), not text.length()-1. + float lo_width = 0; + float hi_width = text_width; const base::i18n::TextDirection text_direction = GetTextDirection(text); - for (size_t guess = (lo + hi) / 2; lo <= hi; guess = (lo + hi) / 2) { + while (lo <= hi) { + // Linearly interpolate between |lo| and |hi|, which correspond to widths + // of |lo_width| and |hi_width| to estimate at what position + // |available_width| would be at. Because |lo_width| and |hi_width| are + // both estimates (may be off by a little because, for example, |lo_width| + // may have been calculated from |lo| minus one, not |lo|), we clamp to the + // the valid range. + // |last_guess| is merely used to verify that we're not repeating guesses. + const size_t last_guess = guess; + guess = lo + static_cast<size_t>(ToRoundedInt((available_width - lo_width) * + (hi - lo) / + (hi_width - lo_width))); + guess = cc::MathUtil::ClampToRange(guess, lo, hi); + DCHECK_NE(last_guess, guess); + // Restore colors. They will be truncated to size by SetText. render_text->colors_ = colors_; base::string16 new_text = @@ -1469,11 +1501,15 @@ base::string16 RenderText::Elide(const base::string16& text, break; if (guess_width > available_width) { hi = guess - 1; + hi_width = guess_width; // Move back on the loop terminating condition when the guess is too wide. - if (hi < lo) + if (hi < lo) { lo = hi; + lo_width = guess_width; + } } else { lo = guess + 1; + lo_width = guess_width; } } diff --git a/chromium/ui/gfx/render_text.h b/chromium/ui/gfx/render_text.h index 24ea3d4123d..024c73ed1ac 100644 --- a/chromium/ui/gfx/render_text.h +++ b/chromium/ui/gfx/render_text.h @@ -64,6 +64,7 @@ class GFX_EXPORT SkiaTextRenderer { void SetForegroundColor(SkColor foreground); void SetShader(sk_sp<cc::PaintShader> shader); void DrawSelection(const std::vector<Rect>& selection, SkColor color); + // TODO(vmpstr): Change this API to mimic SkCanvas::drawTextBlob instead. virtual void DrawPosText(const SkPoint* pos, const uint16_t* glyphs, size_t glyph_count); diff --git a/chromium/ui/gfx/render_text_harfbuzz.cc b/chromium/ui/gfx/render_text_harfbuzz.cc index 2f0286217a5..07a4bbd44b0 100644 --- a/chromium/ui/gfx/render_text_harfbuzz.cc +++ b/chromium/ui/gfx/render_text_harfbuzz.cc @@ -15,6 +15,7 @@ #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/profiler/scoped_tracker.h" +#include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/trace_event/trace_event.h" @@ -207,8 +208,9 @@ bool IsNewlineSegment(const base::string16& text, // Helper template function for |TextRunHarfBuzz::GetClusterAt()|. |Iterator| // can be a forward or reverse iterator type depending on the text direction. +// Returns true on success, or false if an error is encountered. template <class Iterator> -void GetClusterAtImpl(size_t pos, +bool GetClusterAtImpl(size_t pos, Range range, Iterator elements_begin, Iterator elements_end, @@ -216,10 +218,14 @@ void GetClusterAtImpl(size_t pos, Range* chars, Range* glyphs) { Iterator element = std::upper_bound(elements_begin, elements_end, pos); + if (element == elements_begin) { + *chars = range; + *glyphs = Range(); + return false; + } + chars->set_end(element == elements_end ? range.end() : *element); glyphs->set_end(reversed ? elements_end - element : element - elements_begin); - - DCHECK(element != elements_begin); while (--element != elements_begin && *element == *(element - 1)); chars->set_start(*element); glyphs->set_start( @@ -231,6 +237,7 @@ void GetClusterAtImpl(size_t pos, DCHECK(!chars->is_empty()); DCHECK(!glyphs->is_reversed()); DCHECK(!glyphs->is_empty()); + return true; } // Returns the line segment index for the |line|, |text_x| pair. |text_x| is @@ -680,24 +687,37 @@ size_t TextRunHarfBuzz::CountMissingGlyphs() const { void TextRunHarfBuzz::GetClusterAt(size_t pos, Range* chars, Range* glyphs) const { - DCHECK(range.Contains(Range(pos, pos + 1))); DCHECK(chars); DCHECK(glyphs); - if (glyph_count == 0) { + bool success = true; + if (glyph_count == 0 || !range.Contains(Range(pos, pos + 1))) { *chars = range; *glyphs = Range(); - return; + success = false; } if (is_rtl) { - GetClusterAtImpl(pos, range, glyph_to_char.rbegin(), glyph_to_char.rend(), - true, chars, glyphs); - return; + success &= GetClusterAtImpl(pos, range, glyph_to_char.rbegin(), + glyph_to_char.rend(), true, chars, glyphs); + } else { + success &= GetClusterAtImpl(pos, range, glyph_to_char.begin(), + glyph_to_char.end(), false, chars, glyphs); } - GetClusterAtImpl(pos, range, glyph_to_char.begin(), glyph_to_char.end(), - false, chars, glyphs); + if (!success) { + std::string glyph_to_char_string; + for (size_t i = 0; i < glyph_count && i < glyph_to_char.size(); ++i) { + glyph_to_char_string += base::SizeTToString(i) + "->" + + base::UintToString(glyph_to_char[i]) + ", "; + } + LOG(ERROR) << " TextRunHarfBuzz error, please report at crbug.com/724880:" + << " range: " << range.ToString() << ", rtl: " << is_rtl << "," + << " level: '" << level << "', script: " << script << "," + << " font: '" << font.GetActualFontNameForTesting() << "'," + << " glyph_count: " << glyph_count << ", pos: " << pos << "," + << " glyph_to_char: " << glyph_to_char_string; + } } RangeF TextRunHarfBuzz::GetGraphemeBounds(RenderTextHarfBuzz* render_text, diff --git a/chromium/ui/gfx/render_text_unittest.cc b/chromium/ui/gfx/render_text_unittest.cc index 72b1673b51b..1eee0004e3f 100644 --- a/chromium/ui/gfx/render_text_unittest.cc +++ b/chromium/ui/gfx/render_text_unittest.cc @@ -24,7 +24,7 @@ #include "base/test/scoped_task_environment.h" #include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" -#include "third_party/skia/include/core/SkRefCnt.h" +#include "third_party/skia/include/core/SkFontStyle.h" #include "third_party/skia/include/core/SkSurface.h" #include "third_party/skia/include/core/SkTypeface.h" #include "ui/gfx/break_list.h" @@ -70,7 +70,7 @@ class RenderTextTestApi { // Callers should ensure that the associated RenderText object is a // RenderTextHarfBuzz instance. - internal::TextRunList* GetHarfBuzzRunList() { + const internal::TextRunList* GetHarfBuzzRunList() const { RenderTextHarfBuzz* render_text = static_cast<RenderTextHarfBuzz*>(render_text_); return render_text->GetRunList(); @@ -428,6 +428,14 @@ class RenderTextTest : public testing::Test, renderer_(canvas()) {} protected: + bool IsWin8Plus() const { +#if defined(OS_WIN) + return base::win::GetVersion() >= base::win::VERSION_WIN8; +#else + return false; +#endif + } + std::unique_ptr<RenderText> CreateRenderTextInstance() const { switch (GetParam()) { case RENDER_TEXT_HARFBUZZ: @@ -450,14 +458,43 @@ class RenderTextTest : public testing::Test, void DrawVisualText() { test_api_->DrawVisualText(renderer()); } - internal::TextRunList* GetHarfBuzzRunList() { + const internal::TextRunList* GetHarfBuzzRunList() const { DCHECK_EQ(RENDER_TEXT_HARFBUZZ, GetParam()); return test_api_->GetHarfBuzzRunList(); } + // Converts the current run list into a human-readable string. Can be used in + // test assertions for a readable expectation and failure message. + // + // The string shows the runs in visual order. Each run is enclosed in square + // brackets, and shows the begin and end inclusive logical character position, + // with an arrow indicating the direction of the run. Single-character runs + // just show the character position. + // + // For example, the the logical bidirectional string "abc+\x05d0\x05d1\x05d2" + // (visual string: "abc+אבג") yields "[0->2][3][6<-4]". + std::string GetRunListStructureString() const { + const internal::TextRunList* run_list = GetHarfBuzzRunList(); + std::string result; + for (size_t i = 0; i < run_list->size(); ++i) { + size_t logical_index = run_list->visual_to_logical(i); + const internal::TextRunHarfBuzz& run = *run_list->runs()[logical_index]; + if (run.range.length() == 1) { + result.append(base::StringPrintf("[%d]", run.range.start())); + } else if (run.is_rtl) { + result.append(base::StringPrintf("[%d<-%d]", run.range.end() - 1, + run.range.start())); + } else { + result.append(base::StringPrintf("[%d->%d]", run.range.start(), + run.range.end() - 1)); + } + } + return result; + } + // Returns a vector of text fragments corresponding to the current list of // text runs. - std::vector<base::string16> GetRuns() { + std::vector<base::string16> GetRunListStrings() const { std::vector<base::string16> runs_as_text; const std::vector<RenderText::FontSpan> spans = render_text_->GetFontSpansForTesting(); @@ -468,11 +505,11 @@ class RenderTextTest : public testing::Test, return runs_as_text; } - // Sets the text to |text|, then returns GetRuns(). + // Sets the text to |text|, then returns GetRunListStrings(). std::vector<base::string16> RunsFor(const base::string16& text) { render_text_->SetText(text); test_api()->EnsureLayout(); - return GetRuns(); + return GetRunListStrings(); } void ResetRenderTextInstance() { @@ -968,43 +1005,58 @@ TEST_P(RenderTextTest, ElidedText) { const wchar_t* display_text; const bool elision_expected; } cases[] = { - // Strings shorter than the elision width should be laid out in full. - { L"", L"" , false }, - { L"M", L"" , false }, - { L" . ", L" . " , false }, - { kWeak, kWeak , false }, - { kLtr, kLtr , false }, - { kLtrRtl, kLtrRtl , false }, - { kLtrRtlLtr, kLtrRtlLtr, false }, - { kRtl, kRtl , false }, - { kRtlLtr, kRtlLtr , false }, - { kRtlLtrRtl, kRtlLtrRtl, false }, - // Strings as long as the elision width should be laid out in full. - { L"012ab", L"012ab" , false }, - // Long strings should be elided with an ellipsis appended at the end. - { L"012abc", L"012a\x2026", true }, - { L"012ab" L"\x5d0\x5d1", L"012a\x2026", true }, - { L"012a" L"\x5d1" L"b", L"012a\x2026", true }, - // No RLM marker added as digits (012) have weak directionality. - { L"01" L"\x5d0\x5d1\x5d2", L"01\x5d0\x5d1\x2026", true }, - // RLM marker added as "ab" have strong LTR directionality. - { L"ab" L"\x5d0\x5d1\x5d2", L"ab\x5d0\x5d1\x2026\x200f", true }, - // Test surrogate pairs. \xd834\xdd1e forms a single code point U+1D11E; - // \xd834\xdd22 forms a second code point U+1D122. The first should be kept; - // the second removed (no surrogate pair should be partially elided). - { L"0123\xd834\xdd1e\xd834\xdd22", L"0123\xd834\xdd1e\x2026", true }, - // Test combining character sequences. U+0915 U+093F forms a compound glyph; - // U+0915 U+0942 forms a second compound glyph. The first should be kept; - // the second removed (no combining sequence should be partially elided). - { L"0123\x0915\x093f\x0915\x0942", L"0123\x0915\x093f\x2026", true }, - // U+05E9 U+05BC U+05C1 U+05B8 forms a four-character compound glyph. Again, - // it should be either fully elided, or not elided at all. If completely - // elided, an LTR Mark (U+200E) should be added. - { L"0\x05e9\x05bc\x05c1\x05b8", L"0\x05e9\x05bc\x05c1\x05b8", false }, - { L"0\x05e9\x05bc\x05c1\x05b8", L"0\x2026\x200E" , true }, - { L"01\x05e9\x05bc\x05c1\x05b8", L"01\x2026\x200E" , true }, - { L"012\x05e9\x05bc\x05c1\x05b8", L"012\x2026\x200E" , true }, - { L"012\xF0\x9D\x84\x9E", L"012\xF0\x2026" , true }, + // Strings shorter than the elision width should be laid out in full. + {L"", L"", false}, + {L"M", L"", false}, + {L" . ", L" . ", false}, + {kWeak, kWeak, false}, + {kLtr, kLtr, false}, + {kLtrRtl, kLtrRtl, false}, + {kLtrRtlLtr, kLtrRtlLtr, false}, + {kRtl, kRtl, false}, + {kRtlLtr, kRtlLtr, false}, + {kRtlLtrRtl, kRtlLtrRtl, false}, + // Strings as long as the elision width should be laid out in full. + {L"012ab", L"012ab", false}, + // Long strings should be elided with an ellipsis appended at the end. + {L"012abc", L"012a\x2026", true}, + {L"012ab" + L"\x5d0\x5d1", + L"012a\x2026", true}, + {L"012a" + L"\x5d1" + L"b", + L"012a\x2026", true}, + // No RLM marker added as digits (012) have weak directionality. + {L"01" + L"\x5d0\x5d1\x5d2", + L"01\x5d0\x5d1\x2026", true}, + // RLM marker added as "ab" have strong LTR directionality. + {L"ab" + L"\x5d0\x5d1\x5d2", + L"ab\x5d0\x5d1\x2026\x200f", true}, + // Test surrogate pairs. \xd834\xdd1e forms a single code point U+1D11E; + // \xd834\xdd22 forms a second code point U+1D122. The first should be + // kept; + // the second removed (no surrogate pair should be partially elided). + {L"0123\xd834\xdd1e\xd834\xdd22", L"0123\xd834\xdd1e\x2026", true}, + // Test combining character sequences. U+0915 U+093F forms a compound + // glyph; + // U+0915 U+0942 forms a second compound glyph. The first should be kept; + // the second removed (no combining sequence should be partially elided). + {L"0123\x0915\x093f\x0915\x0942", L"0123\x0915\x093f\x2026", true}, + // U+05E9 U+05BC U+05C1 U+05B8 forms a four-character compound glyph. + // Again, + // it should be either fully elided, or not elided at all. If completely + // elided, an LTR Mark (U+200E) should be added. + {L"0\x05e9\x05bc\x05c1\x05b8", L"0\x05e9\x05bc\x05c1\x05b8", false}, + {L"0\x05e9\x05bc\x05c1\x05b8", L"0\x2026\x200E", true}, + {L"01\x05e9\x05bc\x05c1\x05b8", L"01\x2026\x200E", true}, + {L"012\x05e9\x05bc\x05c1\x05b8", L"012\x2026\x200E", true}, + // \xF0\x9D\x84\x9E is the UTF-8 bytestring for MUSICAL SYMBOL G CLEF. It + // should not have any internal grapheme boundaries; it should be + // completely removed when eliding. + {L"012\xF0\x9D\x84\x9E", L"012\x2026", true}, }; std::unique_ptr<RenderText> expected_render_text(CreateRenderTextInstance()); @@ -3019,8 +3071,11 @@ TEST_P(RenderTextHarfBuzzTest, Multiline_NormalWidth) { {L"abc defg hijkl", Range(0, 9), Range(9, 14), {3, 1, 4, 1, 5}, 4, true}, {L"qwertyzxcvbn", Range(0, 10), Range(10, 12), {10, 2}, 1, true}, // RTL: should render left-to-right as "<space>43210 \n cba9876". + // Note this used to say "Arabic language", in Arabic, but the last + // character in the string (\x0629) got fancy in an updated Mac font, so + // now the penultimate character repeats. (See "NOTE" below). {L"\x0627\x0644\x0644\x063A\x0629 " - L"\x0627\x0644\x0639\x0631\x0628\x064A\x0629", + L"\x0627\x0644\x0639\x0631\x0628\x064A\x064A", Range(0, 6), Range(6, 13), {1 /* space first */, 5, 7}, @@ -3184,24 +3239,27 @@ TEST_P(RenderTextHarfBuzzTest, Multiline_NewlineCharacterReplacement) { // Ensure horizontal alignment works in multiline mode. TEST_P(RenderTextHarfBuzzTest, Multiline_HorizontalAlignment) { - const struct { + constexpr struct { const wchar_t* const text; const HorizontalAlignment alignment; + const base::i18n::TextDirection display_text_direction; } kTestStrings[] = { - { L"abcdefghij\nhijkl", ALIGN_LEFT }, - { L"nhijkl\nabcdefghij", ALIGN_LEFT }, - // hebrew, 2nd line shorter - { L"\x5d0\x5d1\x5d2\x5d3\x5d4\x5d5\x5d6\x5d7\n\x5d0\x5d1\x5d2\x5d3", - ALIGN_RIGHT }, - // hebrew, 2nd line longer - { L"\x5d0\x5d1\x5d2\x5d3\n\x5d0\x5d1\x5d2\x5d3\x5d4\x5d5\x5d6\x5d7", - ALIGN_RIGHT }, - // arabic, 2nd line shorter - { L"\x62a\x62b\x62c\x62d\x62e\x62f\x630\n\x660\x661\x662\x663\x664", - ALIGN_RIGHT }, - // arabic, 2nd line longer - { L"\x660\x661\x662\x663\x664\n\x62a\x62b\x62c\x62d\x62e\x62f\x630", - ALIGN_RIGHT }, + {L"abcdefghi\nhijk", ALIGN_LEFT, base::i18n::LEFT_TO_RIGHT}, + {L"nhij\nabcdefghi", ALIGN_LEFT, base::i18n::LEFT_TO_RIGHT}, + // Hebrew, 2nd line shorter + {L"\x5d0\x5d1\x5d2\x5d3\x5d4\x5d5\x5d6\x5d7\n\x5d0\x5d1\x5d2\x5d3", + ALIGN_RIGHT, base::i18n::RIGHT_TO_LEFT}, + // Hebrew, 2nd line longer + {L"\x5d0\x5d1\x5d2\x5d3\n\x5d0\x5d1\x5d2\x5d3\x5d4\x5d5\x5d6\x5d7", + ALIGN_RIGHT, base::i18n::RIGHT_TO_LEFT}, + // Arabic, 2nd line shorter. + {L"\x0627\x0627\x0627\x0627\x0627\x0627\x0627\x0627\n\x0627\x0644\x0644" + L"\x063A", + ALIGN_RIGHT, base::i18n::RIGHT_TO_LEFT}, + // Arabic, 2nd line longer. + {L"\x0627\x0644\x0644\x063A\n\x0627\x0627\x0627\x0627\x0627\x0627\x0627" + L"\x0627", + ALIGN_RIGHT, base::i18n::RIGHT_TO_LEFT}, }; const int kGlyphSize = 5; RenderTextHarfBuzz* render_text = GetRenderTextHarfBuzz(); @@ -3211,9 +3269,11 @@ TEST_P(RenderTextHarfBuzzTest, Multiline_HorizontalAlignment) { render_text->SetMultiline(true); for (size_t i = 0; i < arraysize(kTestStrings); ++i) { - SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "] %ls", i, - kTestStrings[i].text)); + SCOPED_TRACE(testing::Message("kTestStrings[") + << i << "] = " << kTestStrings[i].text); render_text->SetText(WideToUTF16(kTestStrings[i].text)); + EXPECT_EQ(kTestStrings[i].display_text_direction, + render_text->GetDisplayTextDirection()); render_text->Draw(canvas()); ASSERT_LE(2u, test_api()->lines().size()); if (kTestStrings[i].alignment == ALIGN_LEFT) { @@ -3224,6 +3284,16 @@ TEST_P(RenderTextHarfBuzzTest, Multiline_HorizontalAlignment) { base::WideToUTF16(kTestStrings[i].text), base::string16(1, '\n'), base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL); ASSERT_EQ(2u, lines.size()); + + // Sanity check the input string lengths match the glyph lengths. + EXPECT_EQ(4u, std::min(lines[0].length(), lines[1].length())); + EXPECT_EQ(8u, std::max(lines[0].length(), lines[1].length())); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); + ASSERT_EQ(3U, run_list->runs().size()); + EXPECT_EQ(lines[0].length(), run_list->runs()[0]->glyph_count); + EXPECT_EQ(1u, run_list->runs()[1]->glyph_count); // \n. + EXPECT_EQ(lines[1].length(), run_list->runs()[2]->glyph_count); + int difference = (lines[0].length() - lines[1].length()) * kGlyphSize; EXPECT_EQ(test_api()->GetAlignmentOffset(0).x() + difference, test_api()->GetAlignmentOffset(1).x()); @@ -3434,14 +3504,12 @@ TEST_P(RenderTextHarfBuzzTest, NewlineWithoutMultilineFlag) { TEST_P(RenderTextHarfBuzzTest, HarfBuzz_HorizontalPositions) { const struct { const wchar_t* const text; - const Range first_run_char_range; - const Range second_run_char_range; - bool is_rtl; + const char* expected_runs; } kTestStrings[] = { - { L"abc\x3042\x3044\x3046\x3048\x304A", Range(0, 3), Range(3, 8), false }, - { L"\x062A\x0641\x0627\x062D" - L"\x05EA\x05E4\x05D5\x05D6\x05D9\x05DA\x05DB\x05DD", - Range(0, 4), Range(4, 12), true }, + {L"abc\x3042\x3044\x3046\x3048\x304A", "[0->2][3->7]"}, + {L"\x062A\x0641\x0627\x062D" + L"\x05EA\x05E4\x05D5\x05D6\x05D9\x05DA\x05DB\x05DD", + "[11<-4][3<-0]"}, }; RenderTextHarfBuzz* render_text = GetRenderTextHarfBuzz(); @@ -3451,26 +3519,16 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_HorizontalPositions) { render_text->SetText(WideToUTF16(kTestStrings[i].text)); test_api()->EnsureLayout(); - const internal::TextRunList* run_list = GetHarfBuzzRunList(); - ASSERT_EQ(2U, run_list->runs().size()); - EXPECT_EQ(kTestStrings[i].first_run_char_range, run_list->runs()[0]->range); - EXPECT_EQ(kTestStrings[i].second_run_char_range, - run_list->runs()[1]->range); - // If it's RTL, the visual order is reversed. - if (kTestStrings[i].is_rtl) { - EXPECT_EQ(1U, run_list->logical_to_visual(0)); - EXPECT_EQ(0U, run_list->logical_to_visual(1)); - } else { - EXPECT_EQ(0U, run_list->logical_to_visual(0)); - EXPECT_EQ(1U, run_list->logical_to_visual(1)); - } + EXPECT_EQ(kTestStrings[i].expected_runs, GetRunListStructureString()); DrawVisualText(); std::vector<TestSkiaTextRenderer::TextLog> text_log; renderer()->GetTextLogAndReset(&text_log); - EXPECT_EQ(2U, text_log.size()); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); + ASSERT_EQ(2U, run_list->size()); + ASSERT_EQ(2U, text_log.size()); // Verifies the DrawText happens in the visual order and left-to-right. // If the text is RTL, the logically first run should be drawn at last. @@ -3538,6 +3596,19 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_Clusters) { } } +// Ensures GetClusterAt does not crash on invalid conditions. crbug.com/724880 +TEST_P(RenderTextHarfBuzzTest, HarfBuzz_NoCrashOnTextRunGetClusterAt) { + internal::TextRunHarfBuzz run((Font())); + run.range = Range(0, 4); + run.glyph_count = 4; + // Construct a |glyph_to_char| map where no glyph maps to the first character. + run.glyph_to_char = {1u, 1u, 2u, 3u}; + + Range chars, glyphs; + // GetClusterAt should not crash asking for the cluster at position 0. + ASSERT_NO_FATAL_FAILURE(run.GetClusterAt(0, &chars, &glyphs)); +} + // Ensure that graphemes with multiple code points do not get split. TEST_P(RenderTextHarfBuzzTest, HarfBuzz_SubglyphGraphemeCases) { const wchar_t* cases[] = { @@ -3558,7 +3629,7 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_SubglyphGraphemeCases) { base::string16 text = WideToUTF16(cases[i]); render_text->SetText(text); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); ASSERT_EQ(1U, run_list->size()); internal::TextRunHarfBuzz* run = run_list->runs()[0].get(); @@ -3635,33 +3706,11 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_RunDirection) { // Get the run list for both display directions. render_text->SetDirectionalityMode(DIRECTIONALITY_FORCE_LTR); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); - ASSERT_EQ(4U, run_list->size()); - EXPECT_TRUE(run_list->runs()[0]->is_rtl); - EXPECT_FALSE(run_list->runs()[1]->is_rtl); - EXPECT_TRUE(run_list->runs()[2]->is_rtl); - EXPECT_FALSE(run_list->runs()[3]->is_rtl); - - // The Latin letters should appear to the right of the other runs. - EXPECT_EQ(2U, run_list->logical_to_visual(0)); - EXPECT_EQ(1U, run_list->logical_to_visual(1)); - EXPECT_EQ(0U, run_list->logical_to_visual(2)); - EXPECT_EQ(3U, run_list->logical_to_visual(3)); + EXPECT_EQ("[7<-6][2->5][1<-0][8->10]", GetRunListStructureString()); render_text->SetDirectionalityMode(DIRECTIONALITY_FORCE_RTL); test_api()->EnsureLayout(); - run_list = GetHarfBuzzRunList(); - ASSERT_EQ(4U, run_list->size()); - EXPECT_TRUE(run_list->runs()[0]->is_rtl); - EXPECT_FALSE(run_list->runs()[1]->is_rtl); - EXPECT_TRUE(run_list->runs()[2]->is_rtl); - EXPECT_FALSE(run_list->runs()[3]->is_rtl); - - // The Latin letters should appear to the left of the other runs. - EXPECT_EQ(3U, run_list->logical_to_visual(0)); - EXPECT_EQ(2U, run_list->logical_to_visual(1)); - EXPECT_EQ(1U, run_list->logical_to_visual(2)); - EXPECT_EQ(0U, run_list->logical_to_visual(3)); + EXPECT_EQ("[8->10][7<-6][2->5][1<-0]", GetRunListStructureString()); } TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByUnicodeBlocks) { @@ -3670,23 +3719,13 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByUnicodeBlocks) { // The '\x25B6' "play character" should break runs. http://crbug.com/278913 render_text->SetText(WideToUTF16(L"x\x25B6y")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); - EXPECT_EQ(ToString16Vec({"x", "▶", "y"}), GetRuns()); - ASSERT_EQ(3U, run_list->size()); - EXPECT_EQ(Range(0, 1), run_list->runs()[0]->range); - EXPECT_EQ(Range(1, 2), run_list->runs()[1]->range); - EXPECT_EQ(Range(2, 3), run_list->runs()[2]->range); + EXPECT_EQ(ToString16Vec({"x", "▶", "y"}), GetRunListStrings()); + EXPECT_EQ("[0][1][2]", GetRunListStructureString()); render_text->SetText(WideToUTF16(L"x \x25B6 y")); test_api()->EnsureLayout(); - run_list = GetHarfBuzzRunList(); - EXPECT_EQ(ToString16Vec({"x", " ", "▶", " ", "y"}), GetRuns()); - ASSERT_EQ(5U, run_list->size()); - EXPECT_EQ(Range(0, 1), run_list->runs()[0]->range); - EXPECT_EQ(Range(1, 2), run_list->runs()[1]->range); - EXPECT_EQ(Range(2, 3), run_list->runs()[2]->range); - EXPECT_EQ(Range(3, 4), run_list->runs()[3]->range); - EXPECT_EQ(Range(4, 5), run_list->runs()[4]->range); + EXPECT_EQ(ToString16Vec({"x", " ", "▶", " ", "y"}), GetRunListStrings()); + EXPECT_EQ("[0][1][2][3][4]", GetRunListStructureString()); } TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByEmoji) { @@ -3697,13 +3736,9 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByEmoji) { // separated. See crbug.com/448909 render_text->SetText(UTF8ToUTF16("x\xF0\x9F\x98\x81y\xE2\x9C\xA8")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); - ASSERT_EQ(4U, run_list->size()); - EXPECT_EQ(Range(0, 1), run_list->runs()[0]->range); - // The length is 2 since U+1F601 is represented as a surrogate pair in UTF16. - EXPECT_EQ(Range(1, 3), run_list->runs()[1]->range); - EXPECT_EQ(Range(3, 4), run_list->runs()[2]->range); - EXPECT_EQ(Range(4, 5), run_list->runs()[3]->range); + EXPECT_EQ(ToString16Vec({"x", "😁", "y", "✨"}), GetRunListStrings()); + // U+1F601 is represented as a surrogate pair in UTF-16. + EXPECT_EQ("[0][1->2][3][4]", GetRunListStructureString()); } TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByAscii) { @@ -3713,11 +3748,9 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_BreakRunsByAscii) { // run from the ASCII period character. render_text->SetText(UTF8ToUTF16("\xF0\x9F\x90\xB1.")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); - ASSERT_EQ(2U, run_list->size()); - // U+1F431 is represented as a surrogate pair in UTF16. - EXPECT_EQ(Range(0, 2), run_list->runs()[0]->range); - EXPECT_EQ(Range(2, 3), run_list->runs()[1]->range); + EXPECT_EQ(ToString16Vec({"🐱", "."}), GetRunListStrings()); + // U+1F431 is represented as a surrogate pair in UTF-16. + EXPECT_EQ("[0->1][2]", GetRunListStructureString()); } TEST_P(RenderTextHarfBuzzTest, GlyphBounds) { @@ -3740,7 +3773,7 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_NonExistentFont) { RenderTextHarfBuzz* render_text = GetRenderTextHarfBuzz(); render_text->SetText(ASCIIToUTF16("test")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); ASSERT_EQ(1U, run_list->size()); internal::TextRunHarfBuzz* run = run_list->runs()[0].get(); ShapeRunWithFont(render_text->text(), Font("TheFontThatDoesntExist", 13), @@ -3836,7 +3869,7 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_UniscribeFallback) { // Korean character "han". render_text->SetText(WideToUTF16(L"\xd55c")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); ASSERT_EQ(1U, run_list->size()); EXPECT_EQ(0U, run_list->runs()[0]->CountMissingGlyphs()); } @@ -3855,7 +3888,7 @@ TEST_P(RenderTextHarfBuzzTest, HarfBuzz_UnicodeFallback) { // Korean character "han". render_text->SetText(WideToUTF16(L"\xd55c")); test_api()->EnsureLayout(); - internal::TextRunList* run_list = GetHarfBuzzRunList(); + const internal::TextRunList* run_list = GetHarfBuzzRunList(); ASSERT_EQ(1U, run_list->size()); EXPECT_EQ(0U, run_list->runs()[0]->CountMissingGlyphs()); } @@ -3921,11 +3954,11 @@ TEST_P(RenderTextTest, TextDoesntClip) { { #if !defined(OS_CHROMEOS) int top_test_height = kTestSize; -#if defined(OS_WIN) - // Windows 8+ draws 1 pixel above the display rect. - if (base::win::GetVersion() >= base::win::VERSION_WIN8) + // Windows 8+ and RenderTextMac (since 10.13) draw 1 pixel above the + // display rect. + if (IsWin8Plus() || GetParam() == RENDER_TEXT_MAC) top_test_height = kTestSize - 1; -#endif // OS_WIN + // TODO(dschuyler): On ChromeOS text draws above the GetStringSize rect. SCOPED_TRACE("TextDoesntClip Top Side"); rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, 0, kCanvasSize.width(), @@ -3935,13 +3968,11 @@ TEST_P(RenderTextTest, TextDoesntClip) { { int bottom_test_y = kTestSize + string_size.height(); int bottom_test_height = kTestSize; -#if defined(OS_WIN) // Windows 8+ draws 1 pixel below the display rect. - if (base::win::GetVersion() >= base::win::VERSION_WIN8) { + if (IsWin8Plus()) { bottom_test_y = kTestSize + string_size.height() + 1; bottom_test_height = kTestSize - 1; } -#endif // OS_WIN SCOPED_TRACE("TextDoesntClip Bottom Side"); rect_buffer.EnsureSolidRect(SK_ColorWHITE, 0, bottom_test_y, kCanvasSize.width(), bottom_test_height); @@ -4094,19 +4125,22 @@ TEST_P(RenderTextTest, StylePropagated) { render_text->SetFontList(font_list); DrawVisualText(); - EXPECT_EQ(SkTypeface::kNormal, GetRendererPaint().getTypeface()->style()); + EXPECT_EQ(SkFontStyle::Normal(), + GetRendererPaint().getTypeface()->fontStyle()); render_text->SetWeight(Font::Weight::BOLD); DrawVisualText(); - EXPECT_EQ(SkTypeface::kBold, GetRendererPaint().getTypeface()->style()); + EXPECT_EQ(SkFontStyle::Bold(), GetRendererPaint().getTypeface()->fontStyle()); render_text->SetStyle(TextStyle::ITALIC, true); DrawVisualText(); - EXPECT_EQ(SkTypeface::kBoldItalic, GetRendererPaint().getTypeface()->style()); + EXPECT_EQ(SkFontStyle::BoldItalic(), + GetRendererPaint().getTypeface()->fontStyle()); render_text->SetWeight(Font::Weight::NORMAL); DrawVisualText(); - EXPECT_EQ(SkTypeface::kItalic, GetRendererPaint().getTypeface()->style()); + EXPECT_EQ(SkFontStyle::Italic(), + GetRendererPaint().getTypeface()->fontStyle()); } // Ensure the painter adheres to RenderText::subpixel_rendering_suppressed(). diff --git a/chromium/ui/gfx/sequential_id_generator.cc b/chromium/ui/gfx/sequential_id_generator.cc index cdfc62c5b21..5a1fb1fcd00 100644 --- a/chromium/ui/gfx/sequential_id_generator.cc +++ b/chromium/ui/gfx/sequential_id_generator.cc @@ -48,6 +48,11 @@ bool SequentialIDGenerator::HasGeneratedIDFor(uint32_t number) const { return number_to_id_.find(number) != number_to_id_.end(); } +void SequentialIDGenerator::MaybeReleaseNumber(uint32_t number) { + if (HasGeneratedIDFor(number)) + ReleaseNumber(number); +} + void SequentialIDGenerator::ReleaseGeneratedID(uint32_t id) { UpdateNextAvailableIDAfterRelease(id); Remove(id, &id_to_number_, &number_to_id_); diff --git a/chromium/ui/gfx/sequential_id_generator.h b/chromium/ui/gfx/sequential_id_generator.h index 53c36859cd4..7e247d3c981 100644 --- a/chromium/ui/gfx/sequential_id_generator.h +++ b/chromium/ui/gfx/sequential_id_generator.h @@ -32,6 +32,9 @@ class GFX_EXPORT SequentialIDGenerator { // |number|. bool HasGeneratedIDFor(uint32_t number) const; + // Removes the ID previously generated for |number| if necessary. + void MaybeReleaseNumber(uint32_t number); + // Removes the generated ID |id| from the internal mapping. Since the ID is // no longer mapped to any number, subsequent calls to |GetGeneratedID()| can // use this ID. diff --git a/chromium/ui/gfx/sequential_id_generator_unittest.cc b/chromium/ui/gfx/sequential_id_generator_unittest.cc index ee0aac61bfa..c728f96927e 100644 --- a/chromium/ui/gfx/sequential_id_generator_unittest.cc +++ b/chromium/ui/gfx/sequential_id_generator_unittest.cc @@ -61,4 +61,14 @@ TEST(SequentialIDGeneratorTest, RemoveMultipleNumbers) { EXPECT_EQ(4U, generator.GetGeneratedID(0)); } +TEST(SequentialIDGeneratorTest, MaybeRemoveNumbers) { + const uint32_t kMinID = 0; + SequentialIDGenerator generator(kMinID); + + EXPECT_EQ(0U, generator.GetGeneratedID(42)); + + generator.MaybeReleaseNumber(42); + EXPECT_FALSE(generator.HasGeneratedIDFor(42)); + generator.MaybeReleaseNumber(42); +} } // namespace ui diff --git a/chromium/ui/gfx/shadow_util.cc b/chromium/ui/gfx/shadow_util.cc index cc1d7ad6042..10fc06b3185 100644 --- a/chromium/ui/gfx/shadow_util.cc +++ b/chromium/ui/gfx/shadow_util.cc @@ -8,6 +8,7 @@ #include <vector> #include "base/lazy_instance.h" +#include "base/memory/ptr_util.h" #include "third_party/skia/include/core/SkDrawLooper.h" #include "third_party/skia/include/core/SkRRect.h" #include "ui/gfx/canvas.h" @@ -95,7 +96,7 @@ const ShadowDetails& ShadowDetails::Get(int elevation, int corner_radius) { ShadowDetails* shadow = &insertion.first->second; shadow->values = ShadowValue::MakeMdShadowValues(elevation); auto* source = new ShadowNineboxSource(shadow->values, corner_radius); - shadow->ninebox_image = ImageSkia(source, source->size()); + shadow->ninebox_image = ImageSkia(base::WrapUnique(source), source->size()); return *shadow; } diff --git a/chromium/ui/gfx/skia_color_space_util.cc b/chromium/ui/gfx/skia_color_space_util.cc index 47f1da259d5..b840f7e62cc 100644 --- a/chromium/ui/gfx/skia_color_space_util.cc +++ b/chromium/ui/gfx/skia_color_space_util.cc @@ -14,94 +14,6 @@ namespace gfx { namespace { -// Evaluate the gradient of the linear component of fn -void SkTransferFnEvalGradientLinear(const SkColorSpaceTransferFn& fn, - float x, - float* d_fn_d_fC_at_x, - float* d_fn_d_fF_at_x) { - *d_fn_d_fC_at_x = x; - *d_fn_d_fF_at_x = 1.f; -} - -// Solve for the parameters fC and fF, given the parameter fD. -void SkTransferFnSolveLinear(SkColorSpaceTransferFn* fn, - const float* x, - const float* t, - size_t n) { - // If this has no linear segment, don't try to solve for one. - if (fn->fD <= 0) { - fn->fC = 1; - fn->fF = 0; - return; - } - - // Let ne_lhs be the left hand side of the normal equations, and let ne_rhs - // be the right hand side. This is a 4x4 matrix, but we will only update the - // upper-left 2x2 sub-matrix. - SkMatrix44 ne_lhs; - SkVector4 ne_rhs; - for (int row = 0; row < 2; ++row) { - for (int col = 0; col < 2; ++col) { - ne_lhs.set(row, col, 0); - } - } - for (int row = 0; row < 4; ++row) - ne_rhs.fData[row] = 0; - - // Add the contributions from each sample to the normal equations. - float x_linear_max = 0; - for (size_t i = 0; i < n; ++i) { - // Ignore points in the nonlinear segment. - if (x[i] >= fn->fD) - continue; - x_linear_max = std::max(x_linear_max, x[i]); - - // Let J be the gradient of fn with respect to parameters C and F, evaluated - // at this point. - float J[2]; - SkTransferFnEvalGradientLinear(*fn, x[i], &J[0], &J[1]); - - // Let r be the residual at this point. - float r = t[i]; - - // Update the normal equations left hand side with the outer product of J - // with itself. - for (int row = 0; row < 2; ++row) { - for (int col = 0; col < 2; ++col) { - ne_lhs.set(row, col, ne_lhs.get(row, col) + J[row] * J[col]); - } - // Update the normal equations right hand side the product of J with the - // residual - ne_rhs.fData[row] += J[row] * r; - } - } - - // If we only have a single x point at 0, that isn't enough to construct a - // linear segment, so add an additional point connecting to the nonlinear - // segment. - if (x_linear_max == 0) { - float J[2]; - SkTransferFnEvalGradientLinear(*fn, fn->fD, &J[0], &J[1]); - float r = SkTransferFnEval(*fn, fn->fD); - for (int row = 0; row < 2; ++row) { - for (int col = 0; col < 2; ++col) { - ne_lhs.set(row, col, ne_lhs.get(row, col) + J[row] * J[col]); - } - ne_rhs.fData[row] += J[row] * r; - } - } - - // Solve the normal equations. - SkMatrix44 ne_lhs_inv; - bool invert_result = ne_lhs.invert(&ne_lhs_inv); - DCHECK(invert_result); - SkVector4 solution = ne_lhs_inv * ne_rhs; - - // Update the transfer function. - fn->fC = solution.fData[0]; - fn->fF = solution.fData[1]; -} - // Evaluate the gradient of the nonlinear component of fn void SkTransferFnEvalGradientNonlinear(const SkColorSpaceTransferFn& fn, float x, @@ -282,9 +194,10 @@ bool SkApproximateTransferFnInternal(const float* x, size_t n, SkColorSpaceTransferFn* fn) { // First, guess at a value of fD. Assume that the nonlinear segment applies - // to all x >= 0.25. This is generally a safe assumption (fD is usually less + // to all x >= 0.15. This is generally a safe assumption (fD is usually less // than 0.1). - fn->fD = 0.25f; + const float kLinearSegmentMaximum = 0.15f; + fn->fD = kLinearSegmentMaximum; // Do a nonlinear regression on the nonlinear segment. Use a number of guesses // for the initial value of fG, because not all values will converge. @@ -322,21 +235,32 @@ bool SkApproximateTransferFnInternal(const float* x, } // Now find the maximum x value where this nonlinear fit is no longer - // accurate or no longer defined. + // accurate, no longer defined, or no longer nonnegative. fn->fD = 0.f; float max_x_where_nonlinear_does_not_fit = -1.f; for (size_t i = 0; i < n; ++i) { + if (x[i] >= kLinearSegmentMaximum) + continue; + + // The nonlinear segment is only undefined when fA * x + fB is + // nonnegative. + float fn_at_xi = -1; + if (fn->fA * x[i] + fn->fB >= 0) + fn_at_xi = SkTransferFnEvalUnclamped(*fn, x[i]); + + // If the value is negative (or undefined), say that the fit was bad. bool nonlinear_fits_xi = true; - if (fn->fA * x[i] + fn->fB < 0) { - // The nonlinear segment is undefined when fA * x + fB is less than 0. + if (fn_at_xi < 0) nonlinear_fits_xi = false; - } else { - // Define "no longer accurate" as "has more than 10% more error than - // the maximum error in the fit segment". - float error_at_xi = std::abs(t[i] - SkTransferFnEval(*fn, x[i])); + + // Compute the error, and define "no longer accurate" as "has more than + // 10% more error than the maximum error in the fit segment". + if (nonlinear_fits_xi) { + float error_at_xi = std::abs(t[i] - fn_at_xi); if (error_at_xi > 1.1f * max_error_in_nonlinear_fit) nonlinear_fits_xi = false; } + if (!nonlinear_fits_xi) { max_x_where_nonlinear_does_not_fit = std::max(max_x_where_nonlinear_does_not_fit, x[i]); @@ -353,7 +277,17 @@ bool SkApproximateTransferFnInternal(const float* x, } // Compute the linear segment, now that we have our definitive fD. - SkTransferFnSolveLinear(fn, x, t, n); + if (fn->fD <= 0) { + // If this has no linear segment, don't try to solve for one. + fn->fC = 1; + fn->fF = 0; + } else { + // Set the linear portion such that it go through the origin and be + // continuous with the nonlinear segment. + float fn_at_fD = SkTransferFnEval(*fn, fn->fD); + fn->fC = fn_at_fD / fn->fD; + fn->fF = 0; + } return true; } diff --git a/chromium/ui/gfx/skia_paint_util.cc b/chromium/ui/gfx/skia_paint_util.cc index c18fbd1002e..2f3e2957232 100644 --- a/chromium/ui/gfx/skia_paint_util.cc +++ b/chromium/ui/gfx/skia_paint_util.cc @@ -5,6 +5,7 @@ #include "ui/gfx/skia_paint_util.h" #include "base/memory/ptr_util.h" +#include "cc/paint/paint_image_builder.h" #include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/effects/SkBlurMaskFilter.h" #include "third_party/skia/include/effects/SkGradientShader.h" @@ -38,8 +39,11 @@ sk_sp<cc::PaintShader> CreateImageRepShaderForScale( shader_scale.setScaleY(local_matrix.getScaleY() / scale); return cc::PaintShader::MakeImage( - SkImage::MakeFromBitmap(image_rep.sk_bitmap()), tile_mode, tile_mode, - &shader_scale); + cc::PaintImageBuilder() + .set_id(cc::PaintImage::kNonLazyStableId) + .set_image(SkImage::MakeFromBitmap(image_rep.sk_bitmap())) + .TakePaintImage(), + tile_mode, tile_mode, &shader_scale); } sk_sp<cc::PaintShader> CreateGradientShader(int start_point, diff --git a/chromium/ui/gfx/vector_icon_types.h b/chromium/ui/gfx/vector_icon_types.h index facc054f7eb..cb6f36d7e9e 100644 --- a/chromium/ui/gfx/vector_icon_types.h +++ b/chromium/ui/gfx/vector_icon_types.h @@ -21,6 +21,8 @@ const int kReferenceSizeDip = 48; enum CommandType { // A new <path> element. For the first path, this is assumed. NEW_PATH, + // Sets the alpha for the current path. + PATH_COLOR_ALPHA, // Sets the color for the current path. PATH_COLOR_ARGB, // Sets the path to clear mode (Skia's kClear_Mode). |