Re: [PATCH 1/4] d3d9/tests: Add a windowed GetFrontBufferData test.
On 6 July 2015 at 23:47, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
--- dlls/d3d9/tests/visual.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+)
Testbot failures aside, GetFrontBufferData() is pretty unreliable. This will (potentially) fail if e.g. a different window is covering the test window. (And the ChildWindowFromPoint() in v2 doesn't help, because it only takes other Wine windows into account.)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-07 um 17:00 schrieb Henri Verbeet:
Testbot failures aside, GetFrontBufferData() is pretty unreliable. This will (potentially) fail if e.g. a different window is covering the test window. (And the ChildWindowFromPoint() in v2 doesn't help, because it only takes other Wine windows into account.) Sure, but I don't think we can fundamentally fix this. The only option I see is not having those tests, which would be fine with me.
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVm+/gAAoJEN0/YqbEcdMwK9sP/jeM8nuwzD7X9EDUf+e/7CHy KygMRqHiNJ97io69Tdp2p4kXdQJITtMNfpO8B7udCryiFyTv3YKhO1nq9isaMnZw ckVHvw1b7g89yDQYIQ0jB0UmXrilvVVWcbf5/OyPZs/F6nloR4Uc+YtCNrmMGVbx wdCn7l0Tso9R4bNveznW18NMcxvPuxhTJfetLlJppXb3bgwSnBysvGYUrXB+JJ1t RuTGBqa5/r5NavKKu72kHOEiBnVk+fHzhpmoBh4cm2gFIrlgG11gFD5mhbgkYnxX UY6NaUtrwuhhyD8/4cSxomSp0myScAGxEhlfyIY+QBX9MMLRiJDFoKOYqMnSE/D4 fJYE+8LxF6LS2uM5pohCOIAQ4ynckYUfQAP5cLEZWKY76HjEep+pey23BHU3AcfG LL9q8baS3wM5IycU9r6oEjbBc2OfXoGl+cR6zoMnMpmzEov04knwwRR3jCr0szLc g+jtTfyzphsSgzVeOHii17NogamYQ1PqJ+8Y+uMt5XAb+uCEsCYPeMoupqSxJB8V epiGIXshDLNoFkBo646cMWB2KviDWpWjaNIfAvB+rwvs2tre/PJUFV3DRdQr6hlv JXZHcCJ2cTRO6LEHHPGTj/nnQOpHFVfKeilDP7eglLB1DZ+u6fcl272wCD4WXWQc ZYHdOjPC9zBEK9qlpCiJ =lBLT -----END PGP SIGNATURE-----
On 7 July 2015 at 17:27, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Am 2015-07-07 um 17:00 schrieb Henri Verbeet:
Testbot failures aside, GetFrontBufferData() is pretty unreliable. This will (potentially) fail if e.g. a different window is covering the test window. (And the ChildWindowFromPoint() in v2 doesn't help, because it only takes other Wine windows into account.) Sure, but I don't think we can fundamentally fix this. The only option I see is not having those tests, which would be fine with me.
I theory, once the shadowfb is moved from ddraw to wined3d we'd never read from the real drawable anymore.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-08 um 09:26 schrieb Henri Verbeet: > I theory, once the shadowfb is moved from ddraw to wined3d we'd > never read from the real drawable anymore. I don't think we want to use the shadowfb unconditionally. It would mean an extra copy (shadow backbuffer -> shadow frontbuffer -> GL_BACK - -> swap as opposed to shadow backbuffer -> GL_BACK -> swap). I don't think a reliable GetFrontBufferData is worth the extra copy, especially since GetFrontbufferData isn't reliable on Windows either (But ChildWindowFromPoint is). We need the shadowfb for ddraw (esp. overlays, but also other corner cases) and swapeffect_flip, although in that case we ideally swap the FBOs and avoid the shadow backbuffer -> shadow frontbuffer blit and just do shadow frontbuffer -> GL_BACK -> swap. In theory we also want the shadow frontbuffer for a correct software cursor, but there are other problems with that. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVnOSyAAoJEN0/YqbEcdMwM7QP/3WPmfIGTxyKaduPvbTa6+YE TC16ZC09iAX8Mgh5EfPLH9OwYohgVf5iVVrsR+wrmMHvo6HIXfxWL+F+bqckNmU7 a8Q21xxIyQ6NzhnUGUicLsv6SU7Z5XaNQl/BeqVLnj+XuYdPEXV8/yVePkFB23V0 YlT2lbEHIj53LcBY406587Pmt4+JpiU5NgEGQAV913OGAAImO4yeiTU/PlYesAGl zb0FGmjGoHTwnY6CnjmluL1IQk0tSDUgm5dN9Pfp+HfdvLjiIAlPGiemRRuPRSUG h/yRsk1dzt4X7ijgFfQ08pc2/pxNGscV5CjQgIc3q30bf5Wv4B67EZ9pEl1Y/mAr YUBCkiEEvss+Qz15FIjWpRcbM8ZuWOIT0Sl8YY/N7WCZfdzA2wTitbbE47qAlKvI aGRCi5nF0dKVSWKQ4Sm2HqsbeUckpDc3k6gBIjxnQDGvrocvIYsaK5KxdDkp+Has EBvNMBMW0izEVfuLJMCzUg/PtxdXMQuWzCERXixvhu8z1WhJPHr+8XpQx8UOcbs1 FNp5X4WdIU4q5olyijkT5XJXoZ3z2Mqoz4Opx1W6nbBRITjFGYV4yJXizz2+tiWb b7nlPew9IUK8R9fGlz5TCUcAhEG4YIlur6lvBlfyA9BLOqZijoEi4QY395KofZ86 XhHUlutdZ/gWQ1OynG9N =XeTQ -----END PGP SIGNATURE-----
On 8 July 2015 at 10:52, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
I don't think we want to use the shadowfb unconditionally. It would mean an extra copy (shadow backbuffer -> shadow frontbuffer -> GL_BACK - -> swap as opposed to shadow backbuffer -> GL_BACK -> swap). I don't think a reliable GetFrontBufferData is worth the extra copy, especially since GetFrontbufferData isn't reliable on Windows either (But ChildWindowFromPoint is).
We need the shadowfb for ddraw (esp. overlays, but also other corner cases) and swapeffect_flip, although in that case we ideally swap the FBOs and avoid the shadow backbuffer -> shadow frontbuffer blit and just do shadow frontbuffer -> GL_BACK -> swap. In theory we also want the shadow frontbuffer for a correct software cursor, but there are other problems with that.
Actually, it would probably be helpful to have an overview of how this is all going to work in the end. E.g., how is ddraw_surface7_Flip() going to work with the new setup?
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-15 um 10:47 schrieb Henri Verbeet:
Actually, it would probably be helpful to have an overview of how this is all going to work in the end. E.g., how is ddraw_surface7_Flip() going to work with the new setup? Which part of this complicated function are you interested in? :-)
I've attached the current status of my work, but it's far from complete. The immediate goal is to make swapchain_blit work with wined3d_surface_blt (patch 13). Yeah, I realize I don't need most of the previous 12 patches for that technically. The current code handles flip in the ddraw and d3d9 version by swapping the GL textures, invalidates all other locations (could move around sysmem as well, but didn't bother yet) and unloads the resource to remove the textures from the FBO cache. The same thing is currently done in flip_surface() in surface.c Especially the FBO cache handling is ugly. Patches 15-17 give some idea how I plan ddraw overlays to work. I used the d3d9 software cursor to experiment with this, but I don't think we want to handle the software cursor in the way patch 15 handles it. It's matching the native behavior, but GL vsync causes major performance problems when updating the SW cursor and patch 17 doesn't really fix this. As you can see in patch 18-20 I hit a wall while trying to eliminate the ddraw shadow frontbuffer. I think I first have to replace the swapchain destroy and recreate in SetCooperativeLevel with a _Reset call, and I'm sure there are many other issues. As such I haven't even bothered yet with the flip target parameter for surface_flip(). -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVpjlAAAoJEN0/YqbEcdMwr/sP/0tsHo4jdww2nszA/IMFF/Lz 5iIXHfS7Ito+Ka39RO4k4YknrTC3IwHWFERrNM5wMimognmKM9B1COLY320bFe78 reBbJ8xgTbqYrk3pr0zjqev80kr2LM3IxKGD9+3PQBmQGfG8VeqsS70KVhnYP1Cj RnoyEIWniGBGXRzg4xKSFARIfKEIulNGungPghFaSz+P5Tu3OB1M/f4X2JlqUzZ0 FwwJL0assbdDhKNygmgnCMYhRm5avhvCO9yuw+k6vTGvvldNBWWL4BU45eDKlN+x hxfuEuXhuKgC7bf8wMpSkZleIl5pWv/NXD+NrZORlSTlEqSoq9oU33MiACbkPqGw ABjjuoX9Egsm5PXg9knbLqdFWOoo22nsTfDhfk6VVAlKUc6hrJdTauVqFEHWvukY drYy1pIkMb04CzOtnYKAdsa872/Xvo7He2II8vaF8ABACu6xADUDCvLV7LqOrbug 8cQnI1gwmyVhkwyUZl6YbCeX9TtPcds+D+zp5ppsc0NUkZs3/hLtgRgK9IWEAG6Y PgJLuWEzBsAkQzZ3tHu/hrZ9TtBPyt2Pv82vBpXfnBUo2WuxG5RwBsM5WXFIDuLf x8EVTdNO4Z30AxYxskyw18/yEOfxYJkEY/z/GZ2GEwLhZMKjVvRm1zXhcneVcCCu gQCLS/ktMK2QNTulpiFQ =4T81 -----END PGP SIGNATURE-----
On 15 July 2015 at 12:43, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
The current code handles flip in the ddraw and d3d9 version by swapping the GL textures, invalidates all other locations (could move around sysmem as well, but didn't bother yet) and unloads the resource to remove the textures from the FBO cache. The same thing is currently done in flip_surface() in surface.c Especially the FBO cache handling is ugly.
I didn't review the entire set, but that's a problem. I think flip_surface() is incredibly ugly, fragile and probably broken in some way. I would very much like it to go away, so I don't think anything that expands its usage is much of an improvement. ddraw_surface7_Flip() doesn't exactly make me happy either, but at least it only swaps complete surfaces and views, instead of depending on all kinds of wined3d internals. Not invalidating basically everything is probably a good thing too.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-15 um 13:23 schrieb Henri Verbeet:
I didn't review the entire set, but that's a problem. I think flip_surface() is incredibly ugly, fragile and probably broken in some way. I would very much like it to go away, so I don't think anything that expands its usage is much of an improvement. ddraw_surface7_Flip() doesn't exactly make me happy either, but at least it only swaps complete surfaces and views, instead of depending on all kinds of wined3d internals. Not invalidating basically everything is probably a good thing too. Yeah, it's ugly, but in the patchset I kept it for now because there are plenty of other problems to solve as well. How we flip the surfaces or surface contents is fairly easy to separate from the rest of the issues we need to fix.
One idea would be to swap the swapchain resource pointers (and render target views if one of the buffers is currently assigned) and have a callback that tells the client libraries to re-fetch the wined3d objects for its front and back buffers. The lucky part is that render targets aren't stored in stateblocks in d3d8/9. I have done absolutely no investigation how well this would work with d3d10/11 yet. The expected problem is that in d3d10 the swapchain textures can be part of any number of views and they all need to be updated. I don't think controlling everything from the client library (the current ddraw_surface7_Flip way) works without having the shadowfb in the client library as well. The other option would be to restructure the FBO cache to store GL textures instead of surfaces to make the GL texture swapping scheme more efficient. That won't fix the remaining ugliness of flip_surface though. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVpkruAAoJEN0/YqbEcdMwBa4P/iplzdKoooOiIW8ia0rOH+qE KkJlTC21UcPlQOPYD9IGkiGQzIlhPYZidf6swZzD+t4a8OEjmE2LytnfHdMinatA yUJhvMCBSBmUKrDGSK/S/BaT47cYlkXfDdmCxUKeGq0tiptXKo+HQh4H2ZVx3ktf x3DBlWw1fzd/57ZqoeojAF7bQ5NyhSCO5dKOcmAgdqxte61Y1/24Lnjt4M1LRlgZ 1aWhTksmA2voF44r9jhrJKSMyHobWWWsvq8X4+1KZD0mcdBnkhdH2MKgYntfqSMc G2GMEQbHuH6+IcnbskB0PTs/XGOSIJu7PpSVvQIdIea3Qj92rYfW/CbqJEcTRg2z 0OlVaST7x+AZvD9PNd0y2ihLxwGUkJ0GSatXBbDzsp3b3h9WsoIwJvEoRZje2uVQ QzVN4+oO1OPYnjYCbz4dARhziPoDlD4ZBHMCb7xjelsblx7CbkPbBxsgrlSJQPIO LEUWJ8om5lQkE1E0zg3AczIUyUcWakZoJnXOdV2nxXadr7geOzWmGQK42+jectg7 L9ZpL78jR9WsumbROQCNVmSZjnqiBWPuO5nbIca8xvZPcXQeCv8mCWzr2F4uskqU g0pCTV78ptKmtVN431yLyrZlV49aN/nekLXyK5+Gd1Rd4xlkUuOU1gtYgPzqGGaF uasFlKSwUiKyC3NG6SX0 =0oMe -----END PGP SIGNATURE-----
On 15 July 2015 at 13:58, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Yeah, it's ugly, but in the patchset I kept it for now because there are plenty of other problems to solve as well. How we flip the surfaces or surface contents is fairly easy to separate from the rest of the issues we need to fix.
I think it's kind of fundamental to the overall design. (And I think I mentioned this before.)
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-15 um 14:41 schrieb Henri Verbeet:
On 15 July 2015 at 13:58, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Yeah, it's ugly, but in the patchset I kept it for now because there are plenty of other problems to solve as well. How we flip the surfaces or surface contents is fairly easy to separate from the rest of the issues we need to fix.
I think it's kind of fundamental to the overall design. (And I think I mentioned this before.) Do you have any thoughts on how this should work? Otherwise my plan is to go for the client lib callback idea (or make the client libs update their pointers automatically after a present call) and write some tests for how dxgi swap effects and multiple backbuffers work and see what happens.
I'll resend the GL drawable clipping and frontbuffer size patches soon and plan to go on to add separate texture pointers in the swapchain for the WGL back and front buffers. The WGL back buffer is needed to use wined3d_surface_blt from the FBO back buffer to the WGL back buffer. The front buffer separation is for consistency for now, but will later be used to separate the shadow front buffer from the real front buffer. I don't expect the flipping solution to influence this because we'll never flip the WGL buffers and at best invalidate their non-drawable locations. (Wrt the polygon offset stuff, I'm waiting for Matteo's test draw work before resending that). -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJVpmglAAoJEN0/YqbEcdMwEoAQAJATAWQ9p1Vb33wKd9XvdIYp 3cMDL/hjgQONVx64t4ACKXQI5/wfmufWqaA112YnAq2DCiARRc//Nl49zBdf74CA WrTgsqlAittA9H0QI542Cd1sIPFlNRxFPO83dzuLAVhsRrUh0M0plCGSJUyWApV9 EQh7COqUVvIPjHDYOX3Hsn5Dlfi0Vi92Fih2wA6VQUVnPobOyVkq+7IbmDiGYaIc dBRg2Ijzfd5KZMsZOdbl/ySjX82BNpnq9puQQf7naTGwGNjBoJiyjNbMBExroUpp p9peqgdlc1V0QWvDBJGksLMbXyEpSPajOR7t2SvTrXsWtjCtmIBL+4x1BZuEcWQv Hl2BYYswFePg1ShNHnzcOyya3VVxTEqhDH/EWty+uQjayDPsHdmb9nq6OhXcSgkY bboKnjaEll6sJMWJpUM8YTtF7UoXNig1eL/WykPjK1AdOhyiWnTKDGvKiNU8rFhK Jwt49O4MyOFtzL6mllhhNHPMsOrWqiHnx5uP7s+3KLazrM8N3EpomxhtaLnNg8tp pQYHQz744ph69mvKBYvEThmL0E6oCFahypIqzu8VruhVis2OPTzvPJBXViGfOwKC hfo9qQiBjOyLfUUv5zgMSxa7bDJP8lHZMgqBxcJaT/4vNhHpub92oV3ptCIzwL/h VY5ajnPijSmVIGLOXPhR =Be+t -----END PGP SIGNATURE-----
On 15 July 2015 at 16:03, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote:
Do you have any thoughts on how this should work? Otherwise my plan is to go for the client lib callback idea (or make the client libs update their pointers automatically after a present call) and write some tests for how dxgi swap effects and multiple backbuffers work and see what happens.
I haven't thought about it in detail. The callback doesn't sound all that attractive, but could be better than flip_surface(). The conclusion may very well turn out to be that it's best to keep this in ddraw.
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2015-07-16 um 20:22 schrieb Henri Verbeet:
The conclusion may very well turn out to be that it's best to keep this in ddraw. Do you mean the shadowfb entirely, or just the flip handling? I guess it should be possible to separate shadowfb and flip handling, at least when we don't require that the front buffer shows up at the end of the backbuffer chain (the ddraw test doesn't test for that afair). It'd require similar code in the other client libraries if we ever find a d3d8+ application that requires correct flipping. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2
iQIcBAEBAgAGBQJVp/nBAAoJEN0/YqbEcdMwokYQAICQu1Hj4R+OsCr4ceSRtD9r zP+sHhjuOnpLZaLL/ShAkg8jwEq2UXYAWZjxW3yyteJ03+A85e8uF8yWD/pARUbA ffb+YKdO4AzpMRdCwiQJh/EUM4CzC90x/NQNvD9EGyv39VihZUOJ45UZtoVSV5fq x7ECyxuMDW8nFNeObGAyABNKupd4nvT9QHSI96AHoWHxJq9PcmnI9IfZ7a4elJQq nOKcmUaUEkiFt3qfWFGEDfDpmsj9Lo1QyvBpare9YEcdxNB4QkuWZCmYS7VTB3Ek 1ZJHj9gtpIbm0uphn+PgwNa2NqfPxMO6ieOfTXcXo1g6O9K4TW1MfVfJrMIHat35 T9sSSNyVNW6LW3b58Ra0th1Hz1yekzSvKmwfuH9pH7yIpKenvBaW3RbGMEIwqRxo cm4Z3GkJ4dmHPvU7bxvLoR8vPefrt0VwgIWLBvHtLCcS1zSaIk5EErivmPyynlXv bbHKmDnbkF45fErJyS2hAAAtQXQyoj41rSdxh4765EbYcscGgYnqaOXlLnADXffi oOSjaaYi4FINlDJdj7YUzPjnnvcG7d+5oQxtLiIxbIBQKmq6rol19AVTM4qikNYe zMc9lE9pnc2GyMMaGkRyAfYS7HJcpXmRNzqPPZflBhl8u4MSXH3DPo0FdK//mGeq S+wDnP8JSp0i4SfKwx9y =QpPg -----END PGP SIGNATURE-----
participants (2)
-
Henri Verbeet -
Stefan Dösinger