Bug 36027 - Content of 3D tests appears in the bottom right corner sometimes
Summary: Content of 3D tests appears in the bottom right corner sometimes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Enrica Casucci
URL:
Keywords: InRadar, PlatformOnly
Depends on:
Blocks:
 
Reported: 2010-03-11 12:11 PST by Enrica Casucci
Modified: 2010-03-12 17:23 PST (History)
3 users (show)

See Also:


Attachments
Patch (14.27 KB, patch)
2010-03-11 14:30 PST, Enrica Casucci
no flags Details | Formatted Diff | Diff
Patch2 (14.35 KB, patch)
2010-03-11 14:51 PST, Enrica Casucci
simon.fraser: review+
Details | Formatted Diff | Diff
Patch3 (364.57 KB, patch)
2010-03-12 11:24 PST, Enrica Casucci
no flags Details | Formatted Diff | Diff
Patch4 (364.44 KB, patch)
2010-03-12 12:26 PST, Enrica Casucci
no flags Details | Formatted Diff | Diff
Patch4 (364.42 KB, patch)
2010-03-12 14:55 PST, Enrica Casucci
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2010-03-11 12:11:53 PST
* STEPS TO REPRODUCE
1. go to http://MillionDollarCu.be/
2. spoof as Safari 4.0.4 for mac in agent
3. while page is loading, move the mouse to the right
4. Refresh page

* RESULTS
content jumps to the bottom right on refresh all the time. It also jumps sometimes if page is loading and I move the mouse.

This bug is reproducible on every version of Windows.
Comment 1 Enrica Casucci 2010-03-11 14:30:57 PST
Created attachment 50540 [details]
Patch
Comment 2 WebKit Review Bot 2010-03-11 14:37:26 PST
Attachment 50540 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/win/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:11:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:13:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:14:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:16:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:17:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:18:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:19:  Line contains tab character.  [whitespace/tab] [5]
WebKit/win/ChangeLog:20:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 14 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Enrica Casucci 2010-03-11 14:51:47 PST
Created attachment 50545 [details]
Patch2

removed tabs from changelog
Comment 4 Simon Fraser (smfr) 2010-03-11 15:02:20 PST
Comment on attachment 50545 [details]
Patch2

> Index: WebCore/manual-tests/win/horizontal-scroll-composited.html
> ===================================================================
> --- WebCore/manual-tests/win/horizontal-scroll-composited.html	(revision 55850)
> +++ WebCore/manual-tests/win/horizontal-scroll-composited.html	(working copy)
> @@ -5,4 +5,4 @@
>  <br />
>  Try scolling right and left and verify that the content is displayed correctly. 
>  </div>
> -<div style="-webkit-transform: translatez(0); width: 1000px; height: 800px; border-style: solid; border-color: Red; border-width: 3px; background-image: url(../resources/apple.jpg); background-repeat:repeat"></div>
> +<div style="-webkit-transform: rotate3d(0,0,1,0deg); width: 1000px; height: 800px; border-style: solid; border-color: Red; border-width: 3px; background-image: url(../resources/apple.jpg); background-repeat:repeat"></div>

Is this change really needed?
> 
> Property changes on: WebCore/manual-tests/win/horizontal-scroll-composited.html
> ___________________________________________________________________
> Added: svn:executable

That's bad.

> Index: WebCore/manual-tests/win/launch-milliondollar.html
> Index: WebCore/manual-tests/win/vertical-scroll-composited.html

I'd like to see these be non-manual tests, so that when we one day have pixel testing on Windows, they'll be useful. They'd go in LayoutTests/compositing/geometry.

> Index: WebCore/platform/graphics/win/WKCACFLayerRenderer.h
> ===================================================================
> --- WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(revision 55850)
> +++ WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(working copy)
> @@ -60,16 +60,17 @@ public:
>      static bool acceleratedCompositingAvailable();
>      static void didFlushContext(CACFContextRef);
>  
> -    void setScrollFrame(const IntRect&);
> +    void setScrollFrame(const IntPoint&, const int, const int);

Use a const IntSize& for height and width.

> +    int m_scrollFrameWidth;
> +    int m_scrollFrameHeight;

Use IntSize here.

r=me with the code changes, and I'd prefer that you made some LayoutTests.
Comment 5 Enrica Casucci 2010-03-11 15:10:15 PST
(In reply to comment #4)
> (From update of attachment 50545 [details])
> > Index: WebCore/manual-tests/win/horizontal-scroll-composited.html
> > ===================================================================
> > --- WebCore/manual-tests/win/horizontal-scroll-composited.html	(revision 55850)
> > +++ WebCore/manual-tests/win/horizontal-scroll-composited.html	(working copy)
> > @@ -5,4 +5,4 @@
> >  <br />
> >  Try scolling right and left and verify that the content is displayed correctly. 
> >  </div>
> > -<div style="-webkit-transform: translatez(0); width: 1000px; height: 800px; border-style: solid; border-color: Red; border-width: 3px; background-image: url(../resources/apple.jpg); background-repeat:repeat"></div>
> > +<div style="-webkit-transform: rotate3d(0,0,1,0deg); width: 1000px; height: 800px; border-style: solid; border-color: Red; border-width: 3px; background-image: url(../resources/apple.jpg); background-repeat:repeat"></div>
> 
> Is this change really needed?
> > 
> > Property changes on: WebCore/manual-tests/win/horizontal-scroll-composited.html
> > ___________________________________________________________________
> > Added: svn:executable
> 
> That's bad.
> 

That wasn't meant to be in the patch. Just my mistake. The executable thing seems to be something I get when creating the patch on Windows sometimes. I'll remove the change to this tests since it is not needed.

> > Index: WebCore/manual-tests/win/launch-milliondollar.html
> > Index: WebCore/manual-tests/win/vertical-scroll-composited.html
> 
> I'd like to see these be non-manual tests, so that when we one day have pixel
> testing on Windows, they'll be useful. They'd go in
> LayoutTests/compositing/geometry.

Is there a way to perform a scroll from a layout test?

> 
> > Index: WebCore/platform/graphics/win/WKCACFLayerRenderer.h
> > ===================================================================
> > --- WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(revision 55850)
> > +++ WebCore/platform/graphics/win/WKCACFLayerRenderer.h	(working copy)
> > @@ -60,16 +60,17 @@ public:
> >      static bool acceleratedCompositingAvailable();
> >      static void didFlushContext(CACFContextRef);
> >  
> > -    void setScrollFrame(const IntRect&);
> > +    void setScrollFrame(const IntPoint&, const int, const int);
> 
> Use a const IntSize& for height and width.
> 
> > +    int m_scrollFrameWidth;
> > +    int m_scrollFrameHeight;
> 
> Use IntSize here.
> 
I'll make these changes. Thanks for showing me.

> r=me with the code changes, and I'd prefer that you made some LayoutTests.
Comment 6 Enrica Casucci 2010-03-12 11:24:37 PST
Created attachment 50613 [details]
Patch3
Comment 7 WebKit Review Bot 2010-03-12 11:28:35 PST
Attachment 50613 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/manual-tests/win/milliondollar.html:1:  One or more unexpected \r (^M) found; better to use only a \n  [whitespace/carriage_return] [1]
Suppressing further [whitespace/carriage_return] reports for this file.
WebCore/manual-tests/win/milliondollar.html:113:  Line contains tab character.  [whitespace/tab] [5]
WARNING: Could not read file. Skipping: "WebCore/manual-tests/win/horizontal-scroll-composited.html"
Total errors found: 138 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Enrica Casucci 2010-03-12 12:26:56 PST
Created attachment 50621 [details]
Patch4
Comment 9 WebKit Review Bot 2010-03-12 12:29:45 PST
Attachment 50621 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/manual-tests/win/milliondollar.html:113:  Line contains tab character.  [whitespace/tab] [5]
WARNING: Could not read file. Skipping: "WebCore/manual-tests/win/horizontal-scroll-composited.html"
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Enrica Casucci 2010-03-12 14:55:07 PST
Created attachment 50634 [details]
Patch4
Comment 11 Enrica Casucci 2010-03-12 15:12:50 PST
Committed revision 55941.
Comment 12 Eric Seidel (no email) 2010-03-12 17:21:20 PST
Looks like this broke the windows build:
http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/11250
Comment 13 Eric Seidel (no email) 2010-03-12 17:21:38 PST
WebContextMenuClient.cpp
..\WebView.cpp(739) : error C2065: 'm_layerRenderer' : undeclared identifier
..\WebView.cpp(740) : error C2227: left of '->setBackingStoreDirty' must point to class/struct/union/generic type
        type is ''unknown-type''
..\WebView.cpp(790) : error C2227: left of '->setBackingStoreDirty' must point to class/struct/union/generic type
        type is ''unknown-type''
..\WebView.cpp(919) : error C2227: left of '->setBackingStoreDirty' must point to class/struct/union/generic type
        type is ''unknown-type''
Comment 14 Simon Fraser (smfr) 2010-03-12 17:23:32 PST
Looks like we need some #ifdefs