Bug 36978 - Very bad scrolling-performance with the Trackpad at http://www.apple.com/ipad/app-store/
Summary: Very bad scrolling-performance with the Trackpad at http://www.apple.com/ipad...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P2 Normal
Assignee: Simon Fraser (smfr)
URL: http://www.apple.com/ipad/app-store/
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-04-01 13:17 PDT by Mehmet
Modified: 2010-04-02 18:05 PDT (History)
3 users (show)

See Also:


Attachments
Patch (3.15 KB, patch)
2010-04-02 16:44 PDT, Simon Fraser (smfr)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mehmet 2010-04-01 13:17:20 PDT
Hello Webkit-Team,

the two-finger-scrolling-performance is very bad and jerky at following site: http://www.apple.com/ipad/app-store/

Hardware: MacBook 3,1
OS: 10.6.3
Tested with: Safari 4.0.5 and Nightly Build r56912

Thanks in advance.

Mehmet
Comment 1 Sam Weinig 2010-04-01 19:06:48 PDT
<rdar://problem/7820819>
Comment 2 Simon Fraser (smfr) 2010-04-02 16:44:24 PDT
Created attachment 52466 [details]
Patch
Comment 3 Darin Adler 2010-04-02 17:03:38 PDT
Comment on attachment 52466 [details]
Patch

> +        // Check to see if the image changed; we have to do this because the call to
> +        // CGImageCreateCopyWithColorSpace() below can create a new image every time.
> +        if (m_uncorrectedContentsImage && CFEqual(m_uncorrectedContentsImage.get(), newImage))
> +            return;

Wow, does CFEqual actually work on CGImages!? And it compares every bit?

> +        m_uncorrectedContentsImage = m_pendingContentsImage = newImage;

Could you do these on separate lines? It's too easy to overlook the second assignment when it's all on one line.

r=me assuming you tested
Comment 4 Simon Fraser (smfr) 2010-04-02 17:26:08 PDT
(In reply to comment #3)
> (From update of attachment 52466 [details])
> > +        // Check to see if the image changed; we have to do this because the call to
> > +        // CGImageCreateCopyWithColorSpace() below can create a new image every time.
> > +        if (m_uncorrectedContentsImage && CFEqual(m_uncorrectedContentsImage.get(), newImage))
> > +            return;
> 
> Wow, does CFEqual actually work on CGImages!? And it compares every bit?

I don't care about bit-for-bit comparisons; this just checks to see if this image is the same as the last one we set. I guess I could test for pointer equality?

> > +        m_uncorrectedContentsImage = m_pendingContentsImage = newImage;
> 
> Could you do these on separate lines? It's too easy to overlook the second
> assignment when it's all on one line.

Will do.
Comment 5 Simon Fraser (smfr) 2010-04-02 18:05:49 PDT
http://trac.webkit.org/changeset/57039