Bug 36027 – Content of 3D tests appears in the bottom right corner sometimes

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Enrica Casucci]

* 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.

[Enrica Casucci]

Created attachment 50540 [details]
Patch

[WebKit Review Bot]

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.

[Enrica Casucci]

Created attachment 50545 [details]
Patch2

removed tabs from changelog

[Simon Fraser (smfr)]

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.

[Enrica Casucci]

(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.

[Enrica Casucci]

Created attachment 50613 [details]
Patch3

[WebKit Review Bot]

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.

[Enrica Casucci]

Created attachment 50621 [details]
Patch4

[WebKit Review Bot]

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.

[Enrica Casucci]

Created attachment 50634 [details]
Patch4

[Enrica Casucci]

Committed revision 55941.

[Eric Seidel (no email)]

Looks like this broke the windows build:
http://build.webkit.org/builders/Windows%20Release%20%28Build%29/builds/11250

[Eric Seidel (no email)]

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''

[Simon Fraser (smfr)]

Looks like we need some #ifdefs