New D3D textures have landed preffed off. Let's make them default instead of the deprecated ones.

try push with the prefs to get an idea of what's left to fix https://tbpl.mozilla.org/?tree=Try&rev=3c4182977a70

oops, that last try push had talos enabled instead of the tests. Here is a new one https://tbpl.mozilla.org/?tree=Try&rev=11419e6a63a4

Does this mean we have fixed the locking issues with content? And d3d9 textures are all done? (Looks like you've got a failing reftest, btw)

(In reply to Nick Cameron [:nrc] from comment #4) > Does this mean we have fixed the locking issues with content? And d3d9 > textures are all done? New ContentClient is disabled on windows at the moment, we still have the locking issues there. d3d9 textures are all done. > (Looks like you've got a failing reftest, btw) Yeah, I'll fix that before preffing it on

I am still unclear about the reftest: it doesn't always fail (3 out of 7). My best guess right now is that it's related to the fact that I forgot to clear the D3D clients when AllocateForSurface is called with the flag ALLOC_CLEAR_SURFACE. This patch fixes it, we'll see in the next try push if it makes a difference on the reftest.

try push https://tbpl.mozilla.org/?tree=Try&rev=96092f1c6cee

(In reply to Nicolas Silva [:nical] from comment #7) > try push https://tbpl.mozilla.org/?tree=Try&rev=96092f1c6cee the last try push had the wrong set of prefs: https://tbpl.mozilla.org/?tree=Try&rev=0a3591ad037f

Comment on attachment 8359818 [details] [diff] [review] clear the allocated D3D0//11 TextureClient if the flag says so Review of attachment 8359818 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/client/ContentClient.cpp @@ -656,5 @@ > -+ // XXX - We should do something! > -+ // This happened while resizing the window. > -+ NS_WARNING("No DrawTarget for quadrant update!"); > -+ return; > -+ } Is this fixing a mistake from the previous patch or should it not be here?

Could you do a try push with talos too, just to see what happens to perf? (Not saying you should block landing if we do regress, but we do need to know).

(In reply to Nick Cameron [:nrc] from comment #10) > Could you do a try push with talos too, just to see what happens to perf? > (Not saying you should block landing if we do regress, but we do need to > know). In particular, I worry that we will be calling Flush (which is super-expensive) via Unlock (on d3d11) more than previously and that will cause regressions.

(In reply to Nick Cameron [:nrc] from comment #9) > ::: gfx/layers/client/ContentClient.cpp > @@ -656,5 @@ > > -+ // XXX - We should do something! > > -+ // This happened while resizing the window. > > -+ NS_WARNING("No DrawTarget for quadrant update!"); > > -+ return; > > -+ } > > Is this fixing a mistake from the previous patch or should it not be here? It should probably be it's own bug, I forgot to take it out of the patch. I ran into it after resizing the window continuously during several seconds. The BorrwoDrawTarget method does have paths where it returns nullptr so I don't think it could be caused to the changes in this patch (nor the previous one), It's just a bit hard to reproduce.

(In reply to Nicolas Silva [:nical] from comment #12) > > Is this fixing a mistake from the previous patch or should it not be here? > > It should probably be it's own bug, I forgot to take it out of the patch. I > ran into it after resizing the window continuously during several seconds. > The BorrwoDrawTarget method does have paths where it returns nullptr so I > don't think it could be caused to the changes in this patch (nor the > previous one), It's just a bit hard to reproduce. Ignore my previous comment, this is just a rebase that got messed up. It's not part of the final patch.

try push with the pref and talos: https://tbpl.mozilla.org/?tree=Try&rev=4f13d344c6b0

To compare, a talos push without the pref https://tbpl.mozilla.org/?tree=Try&rev=ef85560567ed Since in both cases we don't use the new textures for thebes I don't expect much of a difference.

The comparison between the two try pushes is here: http://compare-talos.mattn.ca/?oldRevs=ef85560567ed&newRev=4f13d344c6b0&server=graphs.mozilla.org&submit=true I launched the jobs several times and saw some really crazy numbers at the beginning but it tends to even out as I run more jobs. I have no idea what to make of the numbers though (especially after the crazy variations I have seen).

I think we should just land this ASAP. It's not currently enabled, and we don't want anyone spending time investigating performance issues with old-textures if it's about to be disabled and then deleted.

try+talos push try: -b o -p win32 -u all -t all

rrr: i meant https://tbpl.mozilla.org/?tree=Try&rev=ae83bf31da71

talos. again. https://tbpl.mozilla.org/?tree=Try&rev=3a21cf277f98

This patch fixes the black rectangle problem with CairoTextureClientD3D9, which was caused by me forgetting to reset mNeedsClear, resulting on the texture being cleared twice, making the region that was not invalidated black. Also fixes a problem with DataTextureSourceD3D9/11 where we would forget to pass the TextureFlags or pass the incorrectly when creating the texture.

This makes new textures on par with the deprecated ones on windows (that is, they have the same failing tests...)

There was an unused variable on some platforms because of #ifdefed code causing build failurs (WError). Fixed: https://hg.mozilla.org/integration/mozilla-inbound/rev/0b596070fe54

https://hg.mozilla.org/mozilla-central/rev/ed6969e4fb6f https://hg.mozilla.org/mozilla-central/rev/0b596070fe54

(In reply to Carsten Book [:Tomcat] from comment #29) > https://hg.mozilla.org/mozilla-central/rev/ed6969e4fb6f > https://hg.mozilla.org/mozilla-central/rev/0b596070fe54 Backed out for causing bug 990027. https://hg.mozilla.org/integration/mozilla-inbound/rev/864067766916

Merge of backout: https://hg.mozilla.org/mozilla-central/rev/864067766916

Fixed as part of bug 982413.

Nical, is there anything QA needs to look out for here?

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #33) > Nical, is there anything QA needs to look out for here? No, thanks. This switched from an architecture to another on a configuration that wasn't shippable in either case (OMTC on Windows) so both had their bugs when we made the switch. The work to make it shippable is happening elsewhere.