Do not attempt to recover from device lost masked by present occluded or display mode changed. (issue 8038044)

25 views
Skip to first unread message

apat...@chromium.org

unread,
Mar 26, 2013, 8:00:50 PM3/26/13
to k...@chromium.org, nic...@transgaming.com, angleproj...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: kbr1, nicolas,

Message:
Sorry about this nicolas. We have new issues on Dev channel.

Description:
Do not attempt to recover from device lost masked by present occluded or
display mode changed.

The call to IDirect3DDevice9::Present in testDeviceLost sometimes never
returns on AMD, resulting in a hang.

Also, on AMD, device lost is sometimes never reported at all, even
through present occluded or display mode changed. I'll need to do
something more robust at the app level. It has something to do with the
Chrome GPU process sandbox. These issues only happen when the sandbox is
enabled.

Cleanup for Surface::recreateAdditionalSwapChains to follow once I
determine this gets us back to a stable place.

Please review this at https://codereview.appspot.com/8038044/

Affected files:
M src/libEGL/Display.cpp


Index: src/libEGL/Display.cpp
===================================================================
--- src/libEGL/Display.cpp (revision 2038)
+++ src/libEGL/Display.cpp (working copy)
@@ -916,34 +916,7 @@

if (mDeviceEx)
{
- HRESULT hr = mDeviceEx->CheckDeviceState(NULL);
- if (hr == S_PRESENT_MODE_CHANGED)
- {
- // Reset the device so that D3D stops reporting
S_PRESENT_MODE_CHANGED. Otherwise it will report
- // it continuously, potentially masking a lost device. D3D
resources are not lost on a mode change with WDDM.
- D3DPRESENT_PARAMETERS presentParameters =
getDefaultPresentParameters();
- mDeviceEx->Reset(&presentParameters);
-
- // Existing swap chains sometimes crash on the next present
after a reset.
- for (SurfaceSet::iterator it = mSurfaceSet.begin(); it !=
mSurfaceSet.end(); ++it)
- {
- (*it)->recreateAdditionalSwapChain();
- }
-
- // Reset will not always cause the device loss to be reported
so issue a dummy present.
- mDeviceEx->Present(NULL, NULL, NULL, NULL);
-
- // Retest the device status to see if the mode change really
indicated a lost device.
- hr = mDeviceEx->CheckDeviceState(NULL);
- }
- else if (hr == S_PRESENT_OCCLUDED)
- {
- // CheckDeviceLost continuously returns S_PRESENT_OCCLUDED
while the screen is locked. Calling Present
- // unmasks the device lost error.
- mDeviceEx->Present(NULL, NULL, NULL, NULL);
- hr = mDeviceEx->CheckDeviceState(NULL);
- }
- isLost = FAILED(hr);
+ isLost = FAILED(mDeviceEx->CheckDeviceState(NULL));
}
else if (mDevice)
{


k...@chromium.org

unread,
Mar 27, 2013, 1:42:00 PM3/27/13
to apat...@chromium.org, nic...@transgaming.com, angleproj...@googlegroups.com, re...@codereview-hr.appspotmail.com
LGTM. Seems necessary to get back to a stable state.


https://codereview.appspot.com/8038044/

nicolas%tra...@gtempaccount.com

unread,
Mar 27, 2013, 3:09:00 PM3/27/13
to apat...@chromium.org, k...@chromium.org, nic...@transgaming.com, angleproj...@googlegroups.com, re...@codereview-hr.appspotmail.com
Too bad it leads to a chain of driver issues. Revert looks fine to me.

https://codereview.appspot.com/8038044/
Reply all
Reply to author
Forward
0 new messages