542280 - Use GetUpdateRgn() on Windows Mobile

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Brian Crowder]

Attachment: One big fix

+++ This bug was initially created as a clone of Bug #540668 +++

I see this issue primarily when scrolling chrome windows, such as the awesomepanel.  Whenever are are using a ::Invalidate*() GDI call, we should be adding the same region to our mInvalidatedRegion member.  Without doing so, we end not blitting enough to properly redraw in WM_PAINT.

Alternatively, we should experiment with using ::GetUpdateRgn() or something comparable as a replacement for ::GetRandomRgn() -- which doesn't exist on WinCE apparently -- instead of manually tracking our invalidated region.

Comment 1

[Brian Crowder]

Comment on attachment 423566 [details]
One big fix

This patch helps a lot, but isn't the whole fix.  I'm going to experiment with ::GetUpdateRgn() next.

Comment 2

[Brian Crowder]

Attachment: w00t

This works well and rips out entirely the code that maintains a "duplicate" invalidated region.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 423634 [details] [diff] [review]
w00t

The latest patch works well. It seems to solve the issues I've seen with panning the awesome panel.  

>@@ -1028,21 +1031,10 @@ PRBool nsWindow::OnPaintImageDDraw16()
>     SetLastError(0); // See http://msdn.microsoft.com/en-us/library/dd145046%28VS.85%29.aspx
>     if (MapWindowPoints(mWnd, 0, (LPPOINT)&renderRect, 2) || 0 == (hr = GetLastError()))
>       hr = glpDDPrimary->Blt(&renderRect, glpDDSecondary, &r, 0, NULL);
>-#ifdef WINCE_WINDOWS_MOBILE
>-    if (FAILED(hr))
>-      // add this rect back to the invalidated region so we'll attempt paint it next time around
>-      mInvalidatedRegion->Union(rects->mRects[i].x, rects->mRects[i].y,
>-                                rects->mRects[i].width, rects->mRects[i].height);
>-#endif
>   }
>   result = PR_TRUE;
> 
> cleanup:
>-#ifdef WINCE_WINDOWS_MOBILE
>-  // re-invalidate the region if we failed.
>-  if (!result)
>-    mInvalidatedRegion->Union(*paintRgnWin.get());
>-#endif
>   ::EndPaint(mWnd, &ps);
>   mPaintDC = nsnull;
>   mPainting = PR_FALSE;

I think we still want to re-invalidate the region or rect if we fail to paint it.

Comment 4

[Brian Crowder]

Attachment: the one to review

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 424026 [details] [diff] [review]
the one to review


>+  paintRgn = ::CreateRectRgn(0, 0, 0, 0);
you shouldn't need to initialize it here, just drop this line

> cleanup:
>-#ifdef WINCE_WINDOWS_MOBILE
>-  // re-invalidate the region if we failed.
>-  if (!result)
>-    mInvalidatedRegion->Union(*paintRgnWin.get());
>-#endif
if result is false, have an NS_ERROR() saying so

Comment 6

[Brian Crowder]

Attachment: argh

Comment 7

[Brian Crowder]

http://hg.mozilla.org/mozilla-central/rev/2c5af96a439b

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c2e813b55946