Using clip() is hurting performance on direct2d

This takes the conservative approach of only using fill() when we're sure it's correct.

Comment on attachment 457165 [details] [diff] [review] Use fill() instead of clip() when appropriate Sure, but seems like a copout :-) Shouldn't we make this work for other operators as well?

> Created attachment 457165 [details] [diff] [review] > Use fill() instead of clip() when appropriate > > if (CurrentState().globalAlpha = 1. shouldn't this be '==' instead of '='?

Comment on attachment 457165 [details] [diff] [review] Use fill() instead of clip() when appropriate Er yes, quite. Also make that "1." a "1.0f", because the "." with no 0 broke my head enough that I didn't see the lack of == :-)

also, isn't it wrong to compare to floating-point numbers since this test will never return true?

We need this for performance parity with IE9.

Any reason why this hasn't landed yet (with the = fix)?

(In reply to comment #7) > Any reason why this hasn't landed yet (with the = fix)? This has also been asked several times on Mozillazine. If anyone has any information, we'd love to hear it :)

This change didn't seem to fix the performance problem and I haven't had a chance to investigate why yet.

Sounds like there might be another clip set higher up the stack.

i think , this is crucial bug ( should be set to blocking2.0:beta3) I am wondering why the performance of d2d in beta 1 could surpass the beta2? does it use clip() or fill() ??

Luke there was another D2D bug that fixed some issues, although slowed the performance last month. This is why your seen a beta slowdown. This bug is open to work on fixing part of the performance regression and hasn't been resolved (see comment 10).

Since the fixing of bug 583033 this fix seems to work and performance seems to be back to normal in my tests. We should probably land it! :)

Tiny update to make it compile on current trunk.

The patch still has: if (CurrentState().globalAlpha = 1. with the single = and the 1. without 0.f. Is that correct?

(In reply to comment #16) > The patch still has: > if (CurrentState().globalAlpha = 1. > with the single = and the 1. without 0.f. > Is that correct? As far as I know that's perfectly legal C++.

(In reply to comment #16) > The patch still has: > if (CurrentState().globalAlpha = 1. > with the single = and the 1. without 0.f. > Is that correct? Ugh, I neglected to notice vlad's earlier review comment. I'll work on fixing that.

Pushed follow-up http://hg.mozilla.org/mozilla-central/rev/ee3ab2be0040.

has anyone tested it with IE benchmark???

(In reply to comment #20) > has anyone tested it with IE benchmark??? After doing a quick test on the IE Fishank benchmark the results were pretty much as before - I am getting consistently good 60fps on 250 again.

For me, the results were improved. With this patch I get nearly twice as much fps as before... :)

It seems to me that after any changes to the d2d performance , the JavaScript performance is affected negatively...

(In reply to comment #20) > has anyone tested it with IE benchmark??? (In reply to comment #23) > It seems to me that after any changes to the d2d performance, the JavaScript > performance is affected negatively.. Fyi. The FishIE performance discussion was actually bug 577069; a bug this blocked. Anything else, file another bug please.

(In reply to comment #24) > (In reply to comment #20) > > has anyone tested it with IE benchmark??? > > (In reply to comment #23) > > It seems to me that after any changes to the d2d performance, the JavaScript > > performance is affected negatively.. > > Fyi. The FishIE performance discussion was actually bug 577069; a bug this > blocked. Anything else, file another bug please. the comment#23 is not about FishIETanks benchmark though, according to my own test, JavaScript performance regressed while trying to do some chromeexperiments.com tests.

why don't we enable the d2d write capability by default on the next installment??

Can you file another bug, no need to add further comments here. Also please, if your not familiar with bugzilla, Your last question is off topic, has nothing to do with this bug and your spamming emails. This is not a general d2d discussion, nor a forum. This bug is resolved and only for a specific code change. If you wish to discuss, I suggest you seek help on Mozillazine forums before making further comments.