New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 45553 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment

Investigate layer cycling during scrolling

Project Member Reported by thakis@chromium.org, Jun 1 2010

Issue description

backing_store_mac contains this code when scrolling happens:

      scoped_cftyperef<CGLayerRef> new_layer(CreateCGLayer());

      // If the current view is in a window, the replacement must be too.
      DCHECK(new_layer);

      CGContextRef layer = CGLayerGetContext(new_layer);
      CGContextDrawLayerAtPoint(layer, CGPointMake(0, 0), cg_layer());
      CGContextSaveGState(layer);
      CGContextClipToRect(layer,
                          CGRectMake(clip_rect.x(),
                                     size().height() - clip_rect.bottom(),
                                     clip_rect.width(),
                                     clip_rect.height()));
      CGContextDrawLayerAtPoint(layer, CGPointMake(dx, -dy), cg_layer());
      CGContextRestoreGState(layer);
      cg_layer_.swap(new_layer);

I don't understand why

      CGContextRef layer = CGLayerGetContext(cg_layer());

      CGContextSaveGState(layer);
      CGContextClipToRect(layer,
                          CGRectMake(clip_rect.x(),
                                     size().height() - clip_rect.bottom(),
                                     clip_rect.width(),
                                     clip_rect.height()));
      CGContextDrawLayerAtPoint(layer, CGPointMake(dx, -dy), cg_layer());
      CGContextRestoreGState(layer);

wouldn't do the same thing. It'd save copying the whole layer over to a new layer on every scroll, so it should be faster.

For some reason, doing the change leaves smudge pixel rows around every now and then, as if the repainted rect is a 
pixel too small, so this needs some investigation.

awalker says there's probably no reason the code is like it is at the moment.
 
Labels: OS-Mac
Could the smudges be a 10.5 or video card specific issue?  On 10.6.3, with the new code, I see a definite improvement (it depends on site).  System CPU usage goes down, too.

Comment 4 by krisr@chromium.org, Jun 2 2010

Labels: -Type-Bug Type-Cleanup Mstone-X
Status: Assigned
comment 3: I wrote a small app that repros the problem on 10.5 (attached), so that seems possible. Would be interesting to know if the repro app works fine on 10.6. I will try to talk to some apple graphics guys at wwdc tomorrow.

If that doesn't work out, I think we should consider rewriting backing_store_mac so that it uses (2d-only) opengl for scrolling (rendering to textures etc).
cglayertest.zip
2.2 MB Download
The demo app works on 10.6.

wwdc guy said that this looks like this is a CoreGraphics bug on 10.5. Near-term, we should do this optimization when we're running on 10.6. Medium term, if we should rewrite this in OpenGL depends on how/when the GPU process stuff will go live.
I like that plan - SL adoption is pretty high, and it's enough of a difference to be worth it.

Comment 8 by bugdro...@gmail.com, Jun 11 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=49442 

------------------------------------------------------------------------
r49442 | thakis@chromium.org | 2010-06-10 13:11:31 -0700 (Thu, 10 Jun 2010) | 6 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/backing_store_mac.mm?r1=49442&r2=49441

Mac: Faster scrolling on 10.6.

BUG= 45553 
TEST=Scrolling up and down still works on 10.5 and 10.6. It uses less %cpu on 10.6.

Review URL: http://codereview.chromium.org/2771010
------------------------------------------------------------------------

Status: Fixed
scrolling on 10.6 is now significantly faster, but due to bugs on 10.5 we can't do the optimization there
Status: Verified


Platform:
  Hostname: testings-mac-mini-3.local
  Mac OS X Version 10.6.4 (Build 10F569)
  Processor: 4 Intel 2.66 GHz
  RAM: 2048 MB

Chrome:
  Chrome version: 6.0.460.0 r51834  <<<Release/Debug>>>
  QuickTime Player: 7.6.6
  QuickTime PlayerX: 114

Labels: -Performance bulkmove Stability-Performance
backing_store_mac contains this code when scrolling happens:

      scoped_cftyperef&lt;CGLayerRef&gt; new_layer(CreateCGLayer());

      // If the current view is in a window, the replacement must be too.
      DCHECK(new_layer);

      CGContextRef layer = CGLayerGetContext(new_layer);
      CGContextDrawLayerAtPoint(layer, CGPointMake(0, 0), cg_layer());
      CGContextSaveGState(layer);
      CGContextClipToRect(layer,
                          CGRectMake(clip_rect.x(),
                                     size().height() - clip_rect.bottom(),
                                     clip_rect.width(),
                                     clip_rect.height()));
      CGContextDrawLayerAtPoint(layer, CGPointMake(dx, -dy), cg_layer());
      CGContextRestoreGState(layer);
      cg_layer_.swap(new_layer);

I don't understand why

      CGContextRef layer = CGLayerGetContext(cg_layer());

      CGContextSaveGState(layer);
      CGContextClipToRect(layer,
                          CGRectMake(clip_rect.x(),
                                     size().height() - clip_rect.bottom(),
                                     clip_rect.width(),
                                     clip_rect.height()));
      CGContextDrawLayerAtPoint(layer, CGPointMake(dx, -dy), cg_layer());
      CGContextRestoreGState(layer);

wouldn't do the same thing. It'd save copying the whole layer over to a new layer on every scroll, so it should be faster.

For some reason, doing the change leaves smudge pixel rows around every now and then, as if the repainted rect is a 
pixel too small, so this needs some investigation.

awalker says there's probably no reason the code is like it is at the moment.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 13 2012

Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 13 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Cleanup -Pri-2 -Area-UI -Stability-Performance Cr-UI Pri-3 Performance Type-Bug

Sign in to add a comment