Closed Bug 593361 Opened 9 years ago Closed 9 years ago

Performance Hit in D2D -FishIE test

Categories

(Core :: Layout, defect, major)

x86
Windows 7
defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta7+

People

(Reporter: jmjjeffery, Assigned: roc)

References

()

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

With today's build, and also noted in the forums yesterday, before the backout, the fish-tank test is severely degraded.
https://bugzilla.mozilla.org/show_bug.cgi?id=579276 Seems likely, but..
These also landed at same time:
https://bugzilla.mozilla.org/show_bug.cgi?id=589175
https://bugzilla.mozilla.org/show_bug.cgi?id=589632
https://bugzilla.mozilla.org/show_bug.cgi?id=590735
https://bugzilla.mozilla.org/show_bug.cgi?id=593224
https://bugzilla.mozilla.org/show_bug.cgi?id=590367
https://bugzilla.mozilla.org/show_bug.cgi?id=592985

Fish-tank is running 24fps at default setting where normally its 48-50fps.
See URL above to test.
Also scrolling on some pages seems/feels not a smooth as before the patches landed.
Everything is alot slower for me not the fishtank test , i can feel the gui being way slower. Looking at mozilla ftp 

1283485308 ok 
1283496473 bad
(In reply to comment #2)
> Everything is alot slower for me not the fishtank test , i can feel the gui
> being way slower. Looking at mozilla ftp 
> 
> 1283485308 ok 
> 1283496473 bad

Ok in cset: 20100902204148 7aaf30721c48
Bad in cset: 20100902234753 4b879b793eb6

Myzar there were two other builds in-between the builds you posted - are they OK also or not tested to narrow the range down more ?
tested now 

1283491196 good
1283495516 bad
I have a wild guess, which is that because the glass area of the chrome is a transparent layer --- and there is text over the transparent part --- every time we paint, we end up repainting the entire chrome.
It's extremely late over here but it's just possible this might help. It might also explode spectacularly. It's completely untested. Seems like the right thing though.

We should probably also make the chrome layer actually be retained. Since it's over a completely transparent background anyway, drawing it onto the backbuffer doesn't give you any increase in quality.

Hmm, actually it occurs to me now that since the backbuffer is always transparent on Windows, drawing non-retained layers directly onto the backbuffer is never a win with D2D, since you won't get subpixel-AA for them anyway. Sigh.
Assignee: nobody → roc
Attachment #471851 - Flags: review?(tnikkel)
blocking2.0: --- → ?
Summary: Performance Hit in D2D -Fish test → Performance Hit in D2D -FishIE test
Comment on attachment 471851 [details] [diff] [review]
total wild guess patch

This seems reasonable. But I'm wondering where the clip on the gfxContext comes from and why it's smaller than the layer's visible region.
Attachment #471851 - Flags: review?(tnikkel) → review+
It's the area of the window that needs to be repainted. Come to think of it, I'm not 100% sure that that clip is set with D2D.
It would be great if someone can try this patch to see if it fixes the problem.
Can I have a link to the tryserver to test this out?
Found your tryserver.  I installed the latest one 20100903101920 4661be23732b.

If this has the patch it doesn't fix the problem.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b6pre) Gecko/20100903 Firefox/4.0b6pre - Build ID: 20100903101920
That try server revision is for a different bug, it doesn't have the patch in this bug.
Just finished a build with this patch, it seems to fix the FishIE test but doesn't fix other performance issues mentioned here. Mousing over bookmarks is still laggy (highlight cannot keep up with mouse cursor). Scrolling performance is hard to pinpoint, it seems to work most of the time but I'm seeing periodic slowdowns.

On the FishIE's 20 fish test, I get 60fps with this patch (without it ~24fps). But if I mouse over bookmarks or other gui elements, fps drops down to 30-40fps, same if I hover over the links on the test page. I don't remember if it has always done that though.
btw, switching from Aero theme to Win 7 Basic theme fixes the issues with bookmarks and significantly the perf hit on the FishIE test when hovering over gui or the links on the page.
Turning off Aero, while a stopgap, is not the answer.
Btw... All you need to do is turn off transparency for your theme in W7 to fix the bookmarks issue.
Turning off Aero does seem to be the only way to fix things.  I'm even back to speed with the Fish Demo.
Sounds like this patch is the right direction. I should be able to sort this out tonight.
blocking2.0: ? → beta6+
The regression almost goes away for the gui with layers.accelerate-all true , but the fish tank test is alot slower with D3D9 layer , but it was so before this regression
Bug 593268 is probably what you need there.
With this patch, we're still seeing a performance hit in FishIE because there is a ThebesLayer containing content overlaying the canvases: a strip along the top and the sidebar on the right containing links. These contain text, so in an effort to make the text get subpixel-AA, we make that ThebesLayer non-retained. This means we have to draw it on every frame.

That wouldn't be so bad except that since the backbuffer supports transparency (because of the glass), we don't get subpixel-AA anyway.
This should fix the performance issues with D2D and glass, by eliminating pointless non-retaining.

We still need to figure out what to do to get subpixel AA back for transparent layers over opaque parts of the window with D2D, of course.
Attachment #472163 - Flags: review?(tnikkel)
Another reason part 1 didn't help much with D2D is that I was right in comment #8: with D2D, our target surface clip is not set to the dirty area of the window. That means every time you mouse over something in the chrome, even with part 1 we would repaint all the chrome. But I guess there's no way around that, since with D2D we have to repaint the entire window, so if chrome's not retained, too bad we have to repaint it all. Or could we limit compositing to just a dirtly subrectangle of the window, and set that subrectangle as a clip rect on the target D2D surface?
BTW here's a layer tree for FishIE:

BasicLayerManager (0x81fa2b0)
  BasicContainerLayer (0x904d4c0) [visible=< (x=0, y=0, w=924, h=1022); >]
    BasicThebesLayer (0x90fb998) [visible=< (x=0, y=0, w=924, h=92); >] // CHROME
    BasicContainerLayer (0x96291c8) [clip=(x=0, y=92, w=924, h=908)] [visible=< (x=0, y=92, w=924, h=908); >] [opaqueContent]
      BasicThebesLayer (0x3fe29d0) [clip=(x=0, y=0, w=0, h=0)] [transform=[ 1 0;
 0 1; 0 92; ]]
      BasicColorLayer (0x99cec60) [clip=(x=0, y=92, w=924, h=908)] [transform=[
1 0; 0 1; 0 92; ]] [visible=< (x=0, y=0, w=924, h=908); >] [opaqueContent] [noText] [noTextOverTransparent] [color=rgba(255, 255, 255, 1)] // Web page background color
      BasicCanvasLayer (0x99d0470) [clip=(x=0, y=92, w=924, h=908)] [transform=[
 1 0; 0 1; 0 92; ]] [visible=< (x=0, y=0, w=924, h=908); >] // Demo canvas #1 (probably tank background)
      BasicCanvasLayer (0x99d0128) [clip=(x=0, y=92, w=924, h=908)] [transform=[
 1 0; 0 1; 0 92; ]] [visible=< (x=0, y=0, w=924, h=908); >] // Demo canvas #2 (probably fish)
      BasicThebesLayer (0x3fe2da8) [transform=[ 1 0; 0 1; 0 92; ]] [visible=< (x=0, y=0, w=924, h=908); >] [noTextOverTransparent] // top-bar and sidebar
      BasicCanvasLayer (0x99d0c18) [clip=(x=0, y=92, w=924, h=908)] [transform=[
 1 0; 0 1; 804 132; ]] [visible=< (x=0, y=0, w=110, h=60); >] // I think the FPS meter
    BasicContainerLayer (0x904d948) [clip=(x=0, y=0, w=924, h=1022)] [visible=<
(x=0, y=1000, w=924, h=22); >] [opaqueContent]
      BasicThebesLayer (0x90fbc28) [visible=< (x=0, y=1000, w=924, h=22); >] [opaqueContent] [noTextOverTransparent] [valid=< (x=0, y=1000, w=924, h=22); >] // chrome status bar

Note that the topbar/sidebar layer has noTextOverTransparent. That means on X, Quartz and GDI we can retain the layer with a transparent buffer and subpixel text works fine. It's only D2D where we have a problem.
While I'm not aware of the fine details here, I'm of the opinion that it's not worth to redraw text just to have subpixel-AA, since it kills the benefit of retained layers where it's most needed.

Normal AA looks good enough for me in those cases; maybe some sort of pref could be added to control whether you want good or fast.
> Normal AA looks good enough for me in those cases; 

Maybe for you because you are not using cleartype at all? I have to disagree completely. We don't have cleartype for nearly a decade now to drop support of it in this area. Besides that Direct2D is also about DirectWrite and therefore a better font rendering with subpixel AA. It would be completely against that to drop it in part of the UI. 

> maybe some sort of prefcould be added to control whether you want good or fast. 

That would be an overkill and nothin a user should have to decide.
(In reply to comment #26)
This is not the pace to discuss this. I *love* subpixel-AA, but when text is drawn over complicated stuff in the fist place (glass, some image), then its advantage is questionable.

Subpixel-AA is disabled in a lot of places in the normal Windows 7 UI for this reason. For example:
- Right column of the start menu (text over glass)
- The whole taskbar, when using modes that show window titles
- The window titles above the aeropeek previews
- The address bar and search bar in Explorer shell windows, and in Internet Explorer 8 (when out of focus and therefore transparent)
I'm speaking of non opaque regions like the bookmarks toolbar. Pretty everything in the main window does not get subpixel AA which is a huge disadvantage using D2D right now. I know that technically the whole window is glass but there are regions where it is covered by solid elements and there should be used subpixel AA.
(In reply to comment #23)
> Another reason part 1 didn't help much with D2D is that I was right in comment
> #8: with D2D, our target surface clip is not set to the dirty area of the
> window. That means every time you mouse over something in the chrome, even with
> part 1 we would repaint all the chrome. But I guess there's no way around that,
> since with D2D we have to repaint the entire window, so if chrome's not
> retained, too bad we have to repaint it all. Or could we limit compositing to
> just a dirtly subrectangle of the window, and set that subrectangle as a clip
> rect on the target D2D surface?

We can, setting the clip to the dirty area is actually what we do to the D2D surface in OnPaint I believe...
Comment on attachment 472163 [details] [diff] [review]
Part 2: if drawing into the backbuffer won't give us subpixel-AA anyway, just retain

After I finally figured out what "backbuffer" meant (need crash course in gfx-fu I guess) this makes sense.
Attachment #472163 - Flags: review?(tnikkel) → review+
Keywords: checkin-needed
Whiteboard: [needs landing]
(In reply to comment #29)
> We can, setting the clip to the dirty area is actually what we do to the D2D
> surface in OnPaint I believe...

Hmm, where?
Oops, Visual Studio strikes again
One of the patches in cset:
http://hg.mozilla.org/mozilla-central/rev/b57dda2ee56d

has further degraded FishIE, now down to 13fps..  will this patch recover that loss also ? 

suspect patch:
https://bugzilla.mozilla.org/show_bug.cgi?id=593275
(In reply to comment #34)
> suspect patch:
> https://bugzilla.mozilla.org/show_bug.cgi?id=593275

As far as I can see, that patch only changes tests, so I don't see how it could be that.
http://hg.mozilla.org/mozilla-central/rev/fd13b6ce36bd turned on D3D, and that does regress canvas performance. Bug 593268 will fix it.
http://hg.mozilla.org/mozilla-central/rev/edd65c542c60
http://hg.mozilla.org/mozilla-central/rev/60e7cd9b8ec2

This should resolve this bug as filed. Please file new bugs for any remaining problems.
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
I see no change is the FishIE test.  Still at 13fps.

Tested using cset: 
http://hg.mozilla.org/mozilla-central/rev/cda6a4cf9839
hmm. What if you turn off D3D or apply the patch in bug 593268?
Turning off D3D does bring the frame rate back up to 44-45fps just a tick slower than before I think, but the fps is not steady, swings between 35-44fps.

Sorry, I'm clueless on how to apply patches.
With D2D enabled and D3D disabled performance is still slightly worse than it was before (47 fps now vs 54 fps before the initial performance hit @500 fish). Should this bug be reopened or a new one filled to fix the remaining performance regression?
I think we should file a tracker bug to track FishIE performance. Make that bug depend on bug 593268. When 593268 lands, reevaluate and file a new bug if performance with D3D+D2D is not satisfactory. Thanks!!!
(In reply to comment #28)
> Pretty
> everything in the main window does not get subpixel AA which is a huge
> disadvantage using D2D right now.

BTW is this actually true? "Normal" Web pages (e.g. a page containing just text and images) should get subpixel AA for the Web content with D2D, with or without D3D enabled.
You need to log in before you can comment on or make changes to this bug.