495298 - DirectDraw render mode on windows mobile doesn't work

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Doug Turner (:dougt)]

setting the preference mozilla.widget.render-mode to 3 (enabling direct draw), leaves fennec in a state where we do not render any pages or show UI.  The window title is displayed, but the rest of the screen is not drawn.  Events (eg mouse events) are processed.  No DDErrors are printed to the console.

Comment 1

[Doug Turner (:dougt)]

Attachment: patch v.1

this needs a bunch of clean up, but let me know what you think.


note that we are painting the entire screen each time.  this is a work around to a crash in pixman composite (bug 498517)

Comment 2

[Doug Turner (:dougt)]

using image 24, blit costs between ~46 ms.
using this approach, blit costs ~19 ms.

Comment 3

[Doug Turner (:dougt)]

those numbers are for painting the entire screen.

Comment 4

[Doug Turner (:dougt)]

(that blit time for this approach includes the time to do the conversion from 24bpp to 565)

Comment 5

[Doug Turner (:dougt)]

> OnPaint [0 0 480 640]
-     1 ms (    1 ms total) - OnPaint [0 0 480 640] [Init]
-    46 ms (   48 ms total) - OnPaint [0 0 480 640] [Dispatch]
-     6 ms (   54 ms total) - OnPaint [0 0 480 640] [Locked]
-     1 ms (   56 ms total) - OnPaint [0 0 480 640] [created pixman images]
-    16 ms (   72 ms total) - OnPaint [0 0 480 640] [composite]
-     0 ms (   73 ms total) - OnPaint [0 0 480 640] [unlock]
-     0 ms (   74 ms total) - OnPaint [0 0 480 640] [sethwnd]
-     0 ms (   75 ms total) - OnPaint [0 0 480 640] [preblt]
-     1 ms (   76 ms total) - OnPaint [0 0 480 640] [Blit]

Sample OnPaint timing.

Comment 6

[Doug Turner (:dougt)]

Attachment: patch v.2

Comment 7

[Doug Turner (:dougt)]

this is missing handling resize events (eg. landscape to portrait)

Comment 8

[Doug Turner (:dougt)]

Attachment: patch v.3

handles rotation.  

odd.  it takes 5x longer to blit in landscape mode than in portrait.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

on file: widget/src/windows/nsWindow.h line 343
>   PRBool                  OnPaintDefault2(HDC aDC = nsnull);

Use more descriptive names here -- OnPaintDefault = OnPaintGeneric,
OnPaintDefault2 = OnPaintDDraw16 or something.  Also, you don't need the
default nsnull for the aDC argument on these, one will always be passed in by
OnPaint.

We don't even really need OnPaint here -- just add the if straight to the
WM_PAINT handling, and let's get rid of that extra function call.



on file: widget/src/windows/nsWindow.cpp line 4875
> 	  gSharedSurfaceSize.width = GetSystemMetrics(SM_CXSCREEN);

Need to null out gSharedSurfaceData here, because the Size no longer
corresponds to it.  Better yet, only null it out if needed -- check if the new
width*height*4 is equal to the old w*h*4.  (This is working right now since I
presume this only changes on rotation, and the area stays the same.)


on file: widget/src/windows/nsWindow.cpp line 5943
>   {

Add a comment here saying why we don't need to create gpDDSurf (because we
don't use the cairo ddraw surface for IMAGE_DDRAW16).


on file: widget/src/windows/nsWindow.cpp line 5964
> }

See above; move this into WM_PAINT handling, after renaming the two functions.

... though I see OnPaint is called from WM_PRINTCLIENT.. so ok, maybe keep
OnPaint as a function just for that, but still rename the other two.

Or even better yet -- at the start of OnPaint, do something like:

if (gRenderMode == RENDER_IMAGE_DDRAW16) {
  return OnPaintImageDDraw16(aDC);
}

and have the rest of the OnPaint body be the normal path.


on file: widget/src/windows/nsWindow.cpp line 6500
>   if ( FAILED(hr)) {

nit: no extra whitspace after (

Comment 10

[Doug Turner (:dougt)]

Attachment: patch v.4

(untested in landscape.  will test tonight/tomorrow)

Comment 11

[Doug Turner (:dougt)]

(rotation is fine with this patch)

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 384552 [details] [diff] [review]
patch v.4

Looks good!

Comment 13

[Doug Turner (:dougt)]

fixed. http://hg.mozilla.org/mozilla-central/rev/0eac7ca971d6

Comment 14

[Doug Turner (:dougt)]

build bustage.  on machines without the right sdk, you will need this:

http://hg.mozilla.org/mozilla-central/rev/3eb25c0c2bfe

Comment 15

[Doug Turner (:dougt)]

the build bustage i mentioned above is actual: d5dd9f7d037d