TextIndicatorWindow is larger than it needs to be, especially when not bouncing

https://bugs.webkit.org/show_bug.cgi?id=139876
<rdar://problem/19311017>

Reviewed by Sam Weinig.

* page/TextIndicator.cpp:
(WebCore::TextIndicator::wantsBounce):
(WebCore::TextIndicator::wantsContentCrossfade):
(WebCore::TextIndicator::wantsFadeIn):
* page/TextIndicator.h:
* page/mac/TextIndicatorWindow.mm:
(-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
(-[WebTextIndicatorView _animationDuration]):
(-[WebTextIndicatorView present]):
(WebCore::TextIndicatorWindow::setTextIndicator):
(-[WebTextIndicatorView _textIndicatorWantsBounce]): Deleted.
(-[WebTextIndicatorView _textIndicatorWantsContentCrossfade]): Deleted.
(-[WebTextIndicatorView _textIndicatorWantsFadeIn]): Deleted.
Move the various switches over TextIndicatorPresentationTransition into TextIndicator.
Adjust the margin; only inflate by the shadow if we're not bouncing.
If we are bouncing, fix the math to more tightly fit the bounce.
Previously we were inflating by way too much, and even when we didn't need to.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@177656 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e81f011172604628fc2fdd3c57f18e31a7a36d02..e9ea49cc947422b29446cb49bebb8db11a087704 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2014-12-22  Timothy Horton  <timothy_horton@apple.com>
+
+        TextIndicatorWindow is larger than it needs to be, especially when not bouncing
+        https://bugs.webkit.org/show_bug.cgi?id=139876
+        <rdar://problem/19311017>
+
+        Reviewed by Sam Weinig.
+
+        * page/TextIndicator.cpp:
+        (WebCore::TextIndicator::wantsBounce):
+        (WebCore::TextIndicator::wantsContentCrossfade):
+        (WebCore::TextIndicator::wantsFadeIn):
+        * page/TextIndicator.h:
+        * page/mac/TextIndicatorWindow.mm:
+        (-[WebTextIndicatorView initWithFrame:textIndicator:margin:]):
+        (-[WebTextIndicatorView _animationDuration]):
+        (-[WebTextIndicatorView present]):
+        (WebCore::TextIndicatorWindow::setTextIndicator):
+        (-[WebTextIndicatorView _textIndicatorWantsBounce]): Deleted.
+        (-[WebTextIndicatorView _textIndicatorWantsContentCrossfade]): Deleted.
+        (-[WebTextIndicatorView _textIndicatorWantsFadeIn]): Deleted.
+        Move the various switches over TextIndicatorPresentationTransition into TextIndicator.
+        Adjust the margin; only inflate by the shadow if we're not bouncing.
+        If we are bouncing, fix the math to more tightly fit the bounce.
+        Previously we were inflating by way too much, and even when we didn't need to.
+
 2014-12-22  Chris Dumez  <cdumez@apple.com>
 
         [iOS] Log using FeatureCounter when a PacheCache entry is not reused because it expired

diff --git a/Source/WebCore/page/TextIndicator.cpp b/Source/WebCore/page/TextIndicator.cpp
index ae04ab03ae35d3760873993b441335ca11bab460..362b9c443a1a861464f6d2a5a97d360a1094f4c0 100644 (file)
--- a/Source/WebCore/page/TextIndicator.cpp
+++ b/Source/WebCore/page/TextIndicator.cpp
@@ -194,5 +194,56 @@ TextIndicator::TextIndicator(const TextIndicatorData& data)
 TextIndicator::~TextIndicator()
 {
 }
+    
+bool TextIndicator::wantsBounce() const
+{
+    switch (m_data.presentationTransition) {
+    case TextIndicatorPresentationTransition::BounceAndCrossfade:
+    case TextIndicatorPresentationTransition::Bounce:
+        return true;
+        
+    case TextIndicatorPresentationTransition::FadeIn:
+    case TextIndicatorPresentationTransition::Crossfade:
+    case TextIndicatorPresentationTransition::None:
+        return false;
+    }
+    
+    return false;
+}
+
+bool TextIndicator::wantsContentCrossfade() const
+{
+    if (!m_data.contentImageWithHighlight)
+        return false;
+    
+    switch (m_data.presentationTransition) {
+    case TextIndicatorPresentationTransition::BounceAndCrossfade:
+    case TextIndicatorPresentationTransition::Crossfade:
+        return true;
+        
+    case TextIndicatorPresentationTransition::Bounce:
+    case TextIndicatorPresentationTransition::FadeIn:
+    case TextIndicatorPresentationTransition::None:
+        return false;
+    }
+    
+    return false;
+}
+
+bool TextIndicator::wantsFadeIn() const
+{
+    switch (m_data.presentationTransition) {
+    case TextIndicatorPresentationTransition::FadeIn:
+        return true;
+        
+    case TextIndicatorPresentationTransition::Bounce:
+    case TextIndicatorPresentationTransition::BounceAndCrossfade:
+    case TextIndicatorPresentationTransition::Crossfade:
+    case TextIndicatorPresentationTransition::None:
+        return false;
+    }
+    
+    return false;
+}
 
 } // namespace WebCore

diff --git a/Source/WebCore/page/TextIndicator.h b/Source/WebCore/page/TextIndicator.h
index a13692b8d41a8ce926ebf1d913accfaee6ff9444..57641381c3b9c8a8e5088a499f9e23b2249a49ac 100644 (file)
--- a/Source/WebCore/page/TextIndicator.h
+++ b/Source/WebCore/page/TextIndicator.h
@@ -81,6 +81,10 @@ public:
     void setPresentationTransition(TextIndicatorPresentationTransition transition) { m_data.presentationTransition = transition; }
 
     TextIndicatorData data() const { return m_data; }
+    
+    bool wantsBounce() const;
+    bool wantsContentCrossfade() const;
+    bool wantsFadeIn() const;
 
 private:
     TextIndicator(const TextIndicatorData&);

diff --git a/Source/WebCore/page/mac/TextIndicatorWindow.mm b/Source/WebCore/page/mac/TextIndicatorWindow.mm
index d041922029e0115e248dca74c2fe9fb03d969746..478ab4dcf9381d87fc09c3e4da5a57785e178e76 100644 (file)
--- a/Source/WebCore/page/mac/TextIndicatorWindow.mm
+++ b/Source/WebCore/page/mac/TextIndicatorWindow.mm
@@ -95,7 +95,7 @@ using namespace WebCore;
     self.wantsLayer = YES;
     self.layer.anchorPoint = CGPointZero;
 
-    bool wantsCrossfade = [self _textIndicatorWantsContentCrossfade];
+    bool wantsCrossfade = _textIndicator->wantsContentCrossfade();
 
     FloatSize contentsImageLogicalSize = _textIndicator->contentImage()->size();
     contentsImageLogicalSize.scale(1 / _textIndicator->contentImageScaleFactor());
@@ -240,64 +240,10 @@ static RetainPtr<CABasicAnimation> createFadeInAnimation(CFTimeInterval duration
     return fadeInAnimation;
 }
 
-- (bool)_textIndicatorWantsBounce
-{
-    switch (_textIndicator->presentationTransition()) {
-    case TextIndicatorPresentationTransition::BounceAndCrossfade:
-    case TextIndicatorPresentationTransition::Bounce:
-        return true;
-
-    case TextIndicatorPresentationTransition::FadeIn:
-    case TextIndicatorPresentationTransition::Crossfade:
-    case TextIndicatorPresentationTransition::None:
-        return false;
-    }
-
-    return false;
-}
-
-- (bool)_textIndicatorWantsContentCrossfade
-{
-    if (!_textIndicator->contentImageWithHighlight())
-        return false;
-
-    switch (_textIndicator->presentationTransition()) {
-    case TextIndicatorPresentationTransition::BounceAndCrossfade:
-    case TextIndicatorPresentationTransition::Crossfade:
-        return true;
-
-    case TextIndicatorPresentationTransition::Bounce:
-    case TextIndicatorPresentationTransition::FadeIn:
-    case TextIndicatorPresentationTransition::None:
-        return false;
-    }
-
-    return false;
-}
-
-- (bool)_textIndicatorWantsFadeIn
-{
-    switch (_textIndicator->presentationTransition()) {
-    case TextIndicatorPresentationTransition::FadeIn:
-        return true;
-
-    case TextIndicatorPresentationTransition::Bounce:
-    case TextIndicatorPresentationTransition::BounceAndCrossfade:
-    case TextIndicatorPresentationTransition::Crossfade:
-    case TextIndicatorPresentationTransition::None:
-        return false;
-    }
-
-    return false;
-}
-
 - (CFTimeInterval)_animationDuration
 {
-    bool wantsBounce = [self _textIndicatorWantsBounce];
-    bool wantsCrossfade = [self _textIndicatorWantsContentCrossfade];
-
-    if (wantsBounce) {
-        if (wantsCrossfade)
+    if (_textIndicator->wantsBounce()) {
+        if (_textIndicator->wantsContentCrossfade())
             return bounceWithCrossfadeAnimationDuration;
         return bounceAnimationDuration;
     }
@@ -307,9 +253,9 @@ static RetainPtr<CABasicAnimation> createFadeInAnimation(CFTimeInterval duration
 
 - (void)present
 {
-    bool wantsBounce = [self _textIndicatorWantsBounce];
-    bool wantsCrossfade = [self _textIndicatorWantsContentCrossfade];
-    bool wantsFadeIn = [self _textIndicatorWantsFadeIn];
+    bool wantsBounce = _textIndicator->wantsBounce();
+    bool wantsCrossfade = _textIndicator->wantsContentCrossfade();
+    bool wantsFadeIn = _textIndicator->wantsFadeIn();
     CFTimeInterval animationDuration = [self _animationDuration];
 
     _hasCompletedAnimation = false;
@@ -437,8 +383,13 @@ void TextIndicatorWindow::setTextIndicator(PassRefPtr<TextIndicator> textIndicat
     if (!m_textIndicator)
         return;
 
-    CGFloat horizontalMargin = std::max(dropShadowBlurRadius * 2 + horizontalBorder, contentRect.size.width * 2);
-    CGFloat verticalMargin = std::max(dropShadowBlurRadius * 2 + verticalBorder, contentRect.size.height * 2);
+    CGFloat horizontalMargin = dropShadowBlurRadius * 2 + horizontalBorder;
+    CGFloat verticalMargin = dropShadowBlurRadius * 2 + verticalBorder;
+    
+    if (m_textIndicator->wantsBounce()) {
+        horizontalMargin = std::max(horizontalMargin, contentRect.size.width * (midBounceScale - 1) + horizontalMargin);
+        verticalMargin = std::max(verticalMargin, contentRect.size.height * (midBounceScale - 1) + verticalMargin);
+    }
 
     contentRect = CGRectInset(contentRect, -horizontalMargin, -verticalMargin);
     NSRect windowContentRect = [NSWindow contentRectForFrameRect:NSIntegralRect(NSRectFromCGRect(contentRect)) styleMask:NSBorderlessWindowMask];