Re: There is no need to offset the source rectangle in the reverse direction before scrolling

[Rein Klazes]

On Tue, 13 Dec 2005 23:53:07 +0800, you wrote:

>Changelog:
>    Dmitry Timoshkov <[EMAIL PROTECTED]>
>    There is no need to offset the source rectangle in the reverse
>    direction before scrolling.
>
>--- cvs/hq/wine/dlls/x11drv/scroll.c   2005-11-22 12:41:07.000000000 +0800
>+++ wine/dlls/x11drv/scroll.c  2005-12-13 23:42:15.000000000 +0800


>     /* Then clip again to get the source rectangle that will remain in the
>      * clipping rect */
>     rcSrc = rcClip;
>-    OffsetRect( &rcSrc, -dx, -dy);
>     IntersectRect( &rcSrc, &rcSrc, &rcClip);

In the first place: with this change the IntersectRect call is a no-op 

I doubt  you understand the reason for "to offset the source rectangle
in the reverse direction":

Scrolling should only occur in the clipping rectangle. When you talk
about the *destination* pixels, a clipping with the cliprect is needed.
Here we are talking about the *source* pixels, from destination to
source is indeed reverse. *that* is what is happening here. 

If you are not convinced: the change is wrong and clipping fails. I have
added a test to confirm that. The test also gives a nice visual idea
what goes wrong, just add a Sleep() or two to get the picture.

Changelog:
dlls/x11drv     : scroll.c
dlls/user/tests : win.c
Restore the previous change to X11DRV_ScrollDC. With a regression test
to confirm it was wrong.

Rein.

--- wine/dlls/x11drv/scroll.c   2005-12-14 13:32:28.000000000 +0100
+++ mywine/dlls/x11drv/scroll.c 2005-12-14 13:53:49.000000000 +0100
@@ -94,22 +94,19 @@ BOOL X11DRV_ScrollDC( HDC hdc, INT dx, I
      * rect are scrolled. So first combine Scroll and Clipping rectangles,
      * if available */
     if( lprcScroll)
-    {
         if( lprcClip)
             IntersectRect( &rcClip, lprcClip, lprcScroll);
         else
             rcClip = *lprcScroll;
-    }
     else
-    {
         if( lprcClip)
             rcClip = *lprcClip;
         else
             GetClipBox( hdc, &rcClip);
-    }
     /* Then clip again to get the source rectangle that will remain in the
      * clipping rect */
     rcSrc = rcClip;
+    OffsetRect( &rcSrc, -dx, -dy);
     IntersectRect( &rcSrc, &rcSrc, &rcClip);
     /* now convert to device coordinates */
     LPtoDP(hdc, (LPPOINT)&rcSrc, 2);
--- wine/dlls/user/tests/win.c  2005-11-30 18:21:30.000000000 +0100
+++ mywine/dlls/user/tests/win.c        2005-12-14 13:47:25.000000000 +0100
@@ -2923,6 +2923,43 @@ static void test_scroll(void)
     DestroyWindow( hwnd);
 }
 
+void test_scrolldc( HWND parent)
+{
+    HDC hdc;
+    HRGN hrgn=CreateRectRgn(0,0,0,0);
+    RECT rc, rc2, rcu, cliprc;
+    HWND hwnd1;
+    COLORREF colr;
+
+    hwnd1 = CreateWindowExA(0, "static", NULL,
+            WS_CHILD| WS_VISIBLE |  WS_BORDER ,
+            25, 50, 100, 100, parent, 0, 0, NULL);
+    ShowWindow( parent, SW_SHOW);
+    UpdateWindow( parent);
+    GetClientRect( hwnd1, &rc);
+    hdc = GetDC( hwnd1);
+    /* paint the upper half of the window black */
+    rc2 = rc;
+    rc2.bottom = ( rc.top + rc.bottom) /2;
+    FillRect( hdc, &rc2, GetStockObject(BLACK_BRUSH));
+    /* clip region is the lower half */
+    cliprc=rc; 
+    cliprc.top = (rc.top + rc.bottom) /2;
+    /* test whether scrolled pixels are properly clipped */ 
+    colr = GetPixel( hdc, (rc.left+rc.right)/2, ( rc.top + rc.bottom) /2 - 1);
+    ok ( colr == 0, "pixel should be black, color is %08lx\n", colr); 
+    /* this scroll should not cause any visible changes */ 
+    ScrollDC( hdc, 5, -20, &rc, &cliprc, hrgn, &rcu);
+    colr = GetPixel( hdc, (rc.left+rc.right)/2, ( rc.top + rc.bottom) /2 - 1);
+    ok ( colr == 0, "pixel should be black, color is %08lx\n", colr); 
+
+    /* clean up */
+    DeleteObject( hrgn);
+    DestroyWindow( hwnd1);
+}
+
+
+
 static void test_params(void)
 {
     HWND hwnd;
@@ -3533,6 +3570,7 @@ START_TEST(win)
     test_validatergn(hwndMain);
     test_nccalcscroll( hwndMain);
     test_scrollvalidate( hwndMain);
+    test_scrolldc( hwndMain);
     test_scroll();
     test_IsWindowUnicode();
     test_vis_rgn(hwndMain);