Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(746)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10990004: Revert 158362 - Support high DPI scrollbar on top level web frame. (Closed)

Created:
8 years, 3 months ago by Avi (use Gerrit)
Modified:
8 years, 3 months ago
Reviewers:
flackr, danakj
CC:
chromium-reviews, cc-bugs_chromium.org
Visibility:
Public.

Description

Revert 158362 - Support high DPI scrollbar on top level web frame. BUG=141389 TEST=Run chrome with --force-device-scale-factor=2 --force-compositing-mode and verify that the top-level scrollbar is high dpi. Reverting: CCLayerTreeHostTestAtomicCommit.runMultiThread broke, this change was the only CC change in the blamelist. Review URL: https://chromiumcodereview.appspot.com/10909255 TBR=flackr@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=158386

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -52 lines) Patch
M cc/CCLayerImpl.h View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/CCLayerImpl.cpp View 1 chunk +0 lines, -9 lines 0 comments Download
M cc/CCScrollbarLayerImpl.cpp View 4 chunks +6 lines, -7 lines 0 comments Download
M cc/LayerChromium.h View 2 chunks +0 lines, -3 lines 0 comments Download
M cc/LayerChromium.cpp View 2 chunks +0 lines, -12 lines 0 comments Download
M cc/ScrollbarLayerChromium.h View 1 chunk +0 lines, -2 lines 0 comments Download
M cc/ScrollbarLayerChromium.cpp View 4 chunks +5 lines, -17 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
Avi (use Gerrit)
8 years, 3 months ago (2012-09-24 21:14:46 UTC) #1
danakj
Link to the breakage? Are we sure the test isn't just being flaky? On Mon, ...
8 years, 3 months ago (2012-09-24 21:17:56 UTC) #2
Avi (use Gerrit)
http://build.chromium.org/p/chromium.win/buildstatus?builder=Win7%20Tests%20%28dbg%29%281%29&number=11263 I pinged flackr on IRC to ask for guidance but got no answer. No ...
8 years, 3 months ago (2012-09-24 21:20:55 UTC) #3
Avi (use Gerrit)
http://build.chromium.org/p/chromium.mac/buildstatus?builder=Mac10.6%20Tests%20%281%29&number=29366 the unit tests failed again elsewhere. Probably not you then. Feel free to recommit. ...
8 years, 3 months ago (2012-09-24 21:33:04 UTC) #4
danakj
FYI http://code.google.com/p/chromium/issues/detail?id=148490 On Mon, Sep 24, 2012 at 5:32 PM, Avi Drissman <avi@chromium.org> wrote: > ...
8 years, 3 months ago (2012-09-24 21:37:44 UTC) #5
Avi (use Gerrit)
8 years, 3 months ago (2012-09-24 21:38:34 UTC) #6 
Good point.

I'll revert the revert.

On Mon, Sep 24, 2012 at 5:37 PM, Dana Jansens <danakj@chromium.org> wrote:

> FYI http://code.google.com/p/chromium/issues/detail?id=148490
>
> On Mon, Sep 24, 2012 at 5:32 PM, Avi Drissman <avi@chromium.org> wrote:
> >
>
http://build.chromium.org/p/chromium.mac/buildstatus?builder=Mac10.6%20Tests%...
> >
> > the unit tests failed again elsewhere. Probably not you then. Feel free
> to
> > recommit.
> >
> > Avi
> >
> >
> > On Mon, Sep 24, 2012 at 5:20 PM, Avi Drissman <avi@chromium.org> wrote:
> >>
> >>
> >>
>
http://build.chromium.org/p/chromium.win/buildstatus?builder=Win7%20Tests%20%...
> >>
> >> I pinged flackr on IRC to ask for guidance but got no answer. No shame
> in
> >> being reverted; if it wasn't really the cause just try again.
> >>
> >> Avi
> >>
> >>
> >> On Mon, Sep 24, 2012 at 5:17 PM, Dana Jansens <danakj@chromium.org>
> wrote:
> >>>
> >>> Link to the breakage? Are we sure the test isn't just being flaky?
> >>>
> >>> On Mon, Sep 24, 2012 at 5:14 PM,  <avi@chromium.org> wrote:
> >>> > Reviewers: flackr,
> >>> >
> >>> > Description:
> >>> > Revert 158362 - Support high DPI scrollbar on top level web frame.
> >>> >
> >>> > BUG=141389
> >>> > TEST=Run chrome with --force-device-scale-factor=2
> >>> > --force-compositing-mode
> >>> > and
> >>> > verify that the top-level scrollbar is high dpi.
> >>> >
> >>> > Reverting: CCLayerTreeHostTestAtomicCommit.runMultiThread broke, this
> >>> > change
> >>> > was
> >>> > the only CC change in the blamelist.
> >>> >
> >>> > Review URL: https://chromiumcodereview.appspot.com/10909255
> >>> >
> >>> > TBR=flackr@chromium.org
> >>> >
> >>> > Please review this at https://codereview.chromium.org/10990004/
> >>> >
> >>> > SVN Base: svn://svn.chromium.org/chrome/trunk/src/
> >>> >
> >>> > Affected files:
> >>> >   M     cc/CCLayerImpl.h
> >>> >   M     cc/CCLayerImpl.cpp
> >>> >   M     cc/CCScrollbarLayerImpl.cpp
> >>> >   M     cc/LayerChromium.h
> >>> >   M     cc/LayerChromium.cpp
> >>> >   M     cc/ScrollbarLayerChromium.h
> >>> >   M     cc/ScrollbarLayerChromium.cpp
> >>> >
> >>> >
> >>> > Index: cc/CCLayerImpl.cpp
> >>> > ===================================================================
> >>> > --- cc/CCLayerImpl.cpp  (revision 158385)
> >>> > +++ cc/CCLayerImpl.cpp  (working copy)
> >>> > @@ -224,15 +224,6 @@
> >>> >      return CCInputHandlerClient::ScrollStarted;
> >>> >  }
> >>> >
> >>> > -IntRect CCLayerImpl::layerRectToContentRect(const WebKit::WebRect&
> >>> > layerRect)
> >>> > -{
> >>> > -    float widthScale = static_cast<float>(contentBounds().width()) /
> >>> > bounds().width();
> >>> > -    float heightScale =
> static_cast<float>(contentBounds().height()) /
> >>> > bounds().height();
> >>> > -    FloatRect contentRect(layerRect.x, layerRect.y, layerRect.width,
> >>> > layerRect.height);
> >>> > -    contentRect.scale(widthScale, heightScale);
> >>> > -    return enclosingIntRect(contentRect);
> >>> > -}
> >>> > -
> >>> >  std::string CCLayerImpl::indentString(int indent)
> >>> >  {
> >>> >      std::string str;
> >>> > Index: cc/CCLayerImpl.h
> >>> > ===================================================================
> >>> > --- cc/CCLayerImpl.h    (revision 158385)
> >>> > +++ cc/CCLayerImpl.h    (working copy)
> >>> > @@ -262,8 +262,6 @@
> >>> >
> >>> >      void appendDebugBorderQuad(CCQuadSink&, const
> CCSharedQuadState*,
> >>> > CCAppendQuadsData&) const;
> >>> >
> >>> > -    IntRect layerRectToContentRect(const WebKit::WebRect&
> layerRect);
> >>> > -
> >>> >      virtual void dumpLayerProperties(std::string*, int indent)
> const;
> >>> >      static std::string indentString(int indent);
> >>> >
> >>> > Index: cc/CCScrollbarLayerImpl.cpp
> >>> > ===================================================================
> >>> > --- cc/CCScrollbarLayerImpl.cpp (revision 158385)
> >>> > +++ cc/CCScrollbarLayerImpl.cpp (working copy)
> >>> > @@ -79,8 +79,7 @@
> >>> >      bool premultipledAlpha = false;
> >>> >      bool flipped = false;
> >>> >      FloatRect uvRect(0, 0, 1, 1);
> >>> > -    IntRect boundsRect(IntPoint(), bounds());
> >>> > -    IntRect contentBoundsRect(IntPoint(), contentBounds());
> >>> > +    IntRect boundsRect(IntPoint(), contentBounds());
> >>> >
> >>> >      CCSharedQuadState* sharedQuadState =
> >>> > quadSink.useSharedQuadState(createSharedQuadState());
> >>> >      appendDebugBorderQuad(quadSink, sharedQuadState,
> appendQuadsData);
> >>> > @@ -91,7 +90,7 @@
> >>> >          thumbRect = WebRect();
> >>> >
> >>> >      if (m_thumbResourceId && !thumbRect.isEmpty()) {
> >>> > -        OwnPtr<CCTextureDrawQuad> quad =
> >>> > CCTextureDrawQuad::create(sharedQuadState,
> >>> > layerRectToContentRect(thumbRect), m_thumbResourceId,
> >>> > premultipledAlpha,
> >>> > uvRect, flipped);
> >>> > +        OwnPtr<CCTextureDrawQuad> quad =
> >>> > CCTextureDrawQuad::create(sharedQuadState, IntRect(thumbRect.x,
> >>> > thumbRect.y,
> >>> > thumbRect.width, thumbRect.height), m_thumbResourceId,
> >>> > premultipledAlpha,
> >>> > uvRect, flipped);
> >>> >          quad->setNeedsBlending();
> >>> >          quadSink.append(quad.release(), appendQuadsData);
> >>> >      }
> >>> > @@ -101,12 +100,12 @@
> >>> >
> >>> >      // We only paint the track in two parts if we were given a
> texture
> >>> > for
> >>> > the forward track part.
> >>> >      if (m_foreTrackResourceId && !foreTrackRect.isEmpty())
> >>> > -        quadSink.append(CCTextureDrawQuad::create(sharedQuadState,
> >>> > layerRectToContentRect(foreTrackRect), m_foreTrackResourceId,
> >>> > premultipledAlpha, toUVRect(foreTrackRect, boundsRect), flipped),
> >>> > appendQuadsData);
> >>> > +        quadSink.append(CCTextureDrawQuad::create(sharedQuadState,
> >>> > IntRect(foreTrackRect.x, foreTrackRect.y, foreTrackRect.width,
> >>> > foreTrackRect.height), m_foreTrackResourceId, premultipledAlpha,
> >>> > toUVRect(foreTrackRect, boundsRect), flipped), appendQuadsData);
> >>> >
> >>> >      // Order matters here: since the back track texture is being
> drawn
> >>> > to
> >>> > the entire contents rect, we must append it after the thumb and
> >>> >      // fore track quads. The back track texture contains (and
> >>> > displays) the
> >>> > buttons.
> >>> > -    if (!contentBoundsRect.isEmpty())
> >>> > -        quadSink.append(CCTextureDrawQuad::create(sharedQuadState,
> >>> > IntRect(contentBoundsRect), m_backTrackResourceId, premultipledAlpha,
> >>> > uvRect, flipped), appendQuadsData);
> >>> > +    if (!boundsRect.isEmpty())
> >>> > +        quadSink.append(CCTextureDrawQuad::create(sharedQuadState,
> >>> > IntRect(boundsRect), m_backTrackResourceId, premultipledAlpha,
> uvRect,
> >>> > flipped), appendQuadsData);
> >>> >  }
> >>> >
> >>> >  void CCScrollbarLayerImpl::didLoseContext()
> >>> > @@ -133,7 +132,7 @@
> >>> >
> >>> >  WebKit::WebSize CCScrollbarLayerImpl::CCScrollbar::size() const
> >>> >  {
> >>> > -    return WebKit::WebSize(m_owner->bounds().width(),
> >>> > m_owner->bounds().height());
> >>> > +    return WebKit::WebSize(m_owner->contentBounds().width(),
> >>> > m_owner->contentBounds().height());
> >>> >  }
> >>> >
> >>> >  bool CCScrollbarLayerImpl::CCScrollbar::enabled() const
> >>> > Index: cc/LayerChromium.cpp
> >>> > ===================================================================
> >>> > --- cc/LayerChromium.cpp        (revision 158385)
> >>> > +++ cc/LayerChromium.cpp        (working copy)
> >>> > @@ -116,15 +116,6 @@
> >>> >          m_layerTreeHost->setNeedsCommit();
> >>> >  }
> >>> >
> >>> > -IntRect LayerChromium::layerRectToContentRect(const WebKit::WebRect&
> >>> > layerRect)
> >>> > -{
> >>> > -    float widthScale = static_cast<float>(contentBounds().width()) /
> >>> > bounds().width();
> >>> > -    float heightScale =
> static_cast<float>(contentBounds().height()) /
> >>> > bounds().height();
> >>> > -    FloatRect contentRect(layerRect.x, layerRect.y, layerRect.width,
> >>> > layerRect.height);
> >>> > -    contentRect.scale(widthScale, heightScale);
> >>> > -    return enclosingIntRect(contentRect);
> >>> > -}
> >>> > -
> >>> >  void LayerChromium::setParent(LayerChromium* layer)
> >>> >  {
> >>> >      ASSERT(!layer || !layer->hasAncestor(this));
> >>> > @@ -631,9 +622,6 @@
> >>> >
> >>> >  void LayerChromium::setBoundsContainPageScale(bool
> >>> > boundsContainPageScale)
> >>> >  {
> >>> > -    for (size_t i = 0; i < m_children.size(); ++i)
> >>> > -
> >>> > m_children[i]->setBoundsContainPageScale(boundsContainPageScale);
> >>> > -
> >>> >      if (boundsContainPageScale == m_boundsContainPageScale)
> >>> >          return;
> >>> >
> >>> > Index: cc/LayerChromium.h
> >>> > ===================================================================
> >>> > --- cc/LayerChromium.h  (revision 158385)
> >>> > +++ cc/LayerChromium.h  (working copy)
> >>> > @@ -232,7 +232,6 @@
> >>> >      void setContentsScale(float);
> >>> >
> >>> >      // When true, the layer's contents are not scaled by the current
> >>> > page
> >>> > scale factor.
> >>> > -    // setBoundsContainPageScale recursively sets the value on all
> >>> > child
> >>> > layers.
> >>> >      void setBoundsContainPageScale(bool);
> >>> >      bool boundsContainPageScale() const { return
> >>> > m_boundsContainPageScale;
> >>> > }
> >>> >
> >>> > @@ -274,8 +273,6 @@
> >>> >
> >>> >      void setNeedsCommit();
> >>> >
> >>> > -    IntRect layerRectToContentRect(const WebKit::WebRect&
> layerRect);
> >>> > -
> >>> >      // This flag is set when layer need repainting/updating.
> >>> >      bool m_needsDisplay;
> >>> >
> >>> > Index: cc/ScrollbarLayerChromium.cpp
> >>> > ===================================================================
> >>> > --- cc/ScrollbarLayerChromium.cpp       (revision 158385)
> >>> > +++ cc/ScrollbarLayerChromium.cpp       (working copy)
> >>> > @@ -131,16 +131,6 @@
> >>> >      WebKit::WebScrollbar::ScrollbarPart m_trackPart;
> >>> >  };
> >>> >
> >>> > -bool ScrollbarLayerChromium::needsContentsScale() const
> >>> > -{
> >>> > -    return true;
> >>> > -}
> >>> > -
> >>> > -IntSize ScrollbarLayerChromium::contentBounds() const
> >>> > -{
> >>> > -    return IntSize(lroundf(bounds().width() * contentsScale()),
> >>> > lroundf(bounds().height() * contentsScale()));
> >>> > -}
> >>> > -
> >>> >  class ScrollbarThumbPainter : public LayerPainterChromium {
> >>> >      WTF_MAKE_NONCOPYABLE(ScrollbarThumbPainter);
> >>> >  public:
> >>> > @@ -223,10 +213,8 @@
> >>> >          return;
> >>> >
> >>> >      // Paint and upload the entire part.
> >>> > -    float widthScale = static_cast<float>(contentBounds().width()) /
> >>> > bounds().width();
> >>> > -    float heightScale =
> static_cast<float>(contentBounds().height()) /
> >>> > bounds().height();
> >>> >      IntRect paintedOpaqueRect;
> >>> > -    painter->prepareToUpdate(rect, rect.size(), widthScale,
> >>> > heightScale,
> >>> > paintedOpaqueRect, stats);
> >>> > +    painter->prepareToUpdate(rect, rect.size(), 1, 1,
> >>> > paintedOpaqueRect,
> >>> > stats);
> >>> >      texture->prepareRect(rect, stats);
> >>> >
> >>> >      IntSize destOffset(0, 0);
> >>> > @@ -252,8 +240,8 @@
> >>> >
> >>> >
> >>> >
>
m_foreTrack->texture()->setRequestPriority(CCPriorityCalculator::uiPriority(drawsToRoot));
> >>> >      }
> >>> >      if (m_thumb) {
> >>> > -        IntSize thumbSize =
> >>> >
> >>> >
> layerRectToContentRect(m_geometry->thumbRect(m_scrollbar.get())).size();
> >>> > -        m_thumb->texture()->setDimensions(thumbSize,
> m_textureFormat);
> >>> > +        WebKit::WebRect thumbRect =
> >>> > m_geometry->thumbRect(m_scrollbar.get());
> >>> > +        m_thumb->texture()->setDimensions(IntSize(thumbRect.width,
> >>> > thumbRect.height), m_textureFormat);
> >>> >
> >>> >
> >>> >
>
m_thumb->texture()->setRequestPriority(CCPriorityCalculator::uiPriority(drawsToRoot));
> >>> >      }
> >>> >  }
> >>> > @@ -266,14 +254,14 @@
> >>> >      createTextureUpdaterIfNeeded();
> >>> >
> >>> >      IntPoint scrollbarOrigin(m_scrollbar->location().x,
> >>> > m_scrollbar->location().y);
> >>> > -    IntRect contentRect =
> >>> > layerRectToContentRect(WebKit::WebRect(scrollbarOrigin.x(),
> >>> > scrollbarOrigin.y(), bounds().width(), bounds().height()));
> >>> > +    IntRect contentRect(scrollbarOrigin, contentBounds());
> >>> >      updatePart(m_backTrackUpdater.get(), m_backTrack.get(),
> >>> > contentRect,
> >>> > queue, stats);
> >>> >      if (m_foreTrack && m_foreTrackUpdater)
> >>> >          updatePart(m_foreTrackUpdater.get(), m_foreTrack.get(),
> >>> > contentRect, queue, stats);
> >>> >
> >>> >      // Consider the thumb to be at the origin when painting.
> >>> >      WebKit::WebRect thumbRect =
> >>> > m_geometry->thumbRect(m_scrollbar.get());
> >>> > -    IntRect originThumbRect =
> >>> > layerRectToContentRect(WebKit::WebRect(0, 0,
> >>> > thumbRect.width, thumbRect.height));
> >>> > +    IntRect originThumbRect = IntRect(0, 0, thumbRect.width,
> >>> > thumbRect.height);
> >>> >      if (!originThumbRect.isEmpty())
> >>> >          updatePart(m_thumbUpdater.get(), m_thumb.get(),
> >>> > originThumbRect,
> >>> > queue, stats);
> >>> >  }
> >>> > Index: cc/ScrollbarLayerChromium.h
> >>> > ===================================================================
> >>> > --- cc/ScrollbarLayerChromium.h (revision 158385)
> >>> > +++ cc/ScrollbarLayerChromium.h (working copy)
> >>> > @@ -29,8 +29,6 @@
> >>> >      virtual ~ScrollbarLayerChromium();
> >>> >
> >>> >      // LayerChromium interface
> >>> > -    virtual bool needsContentsScale() const OVERRIDE;
> >>> > -    virtual IntSize contentBounds() const OVERRIDE;
> >>> >      virtual void setTexturePriorities(const CCPriorityCalculator&)
> >>> > OVERRIDE;
> >>> >      virtual void update(CCTextureUpdateQueue&, const
> >>> > CCOcclusionTracker*,
> >>> > CCRenderingStats&) OVERRIDE;
> >>> >      virtual void setLayerTreeHost(CCLayerTreeHost*) OVERRIDE;
> >>> >
> >>> >
> >>> > --
> >>> >
> >>> >
> >>
> >>
> >
>

Powered by Google App Engine
This is Rietveld 408576698