Bug 36027

Summary: Content of 3D tests appears in the bottom right corner sometimes
Product: WebKit Reporter: Enrica Casucci <enrica>
Component: Layout and RenderingAssignee: Enrica Casucci <enrica>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar, PlatformOnly
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch
none
Patch2
simon.fraser: review+
Patch3
none
Patch4
none
Patch4 simon.fraser: review+

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