ReadPixels recovers from device lost error....

ReadPixels recovers from device lost error.

I found that when I switch desktops, GetRenderTargetData will return D3DERR_DEVICELOST. It is unclear to me whether ReadPixels should report a GL error in this case and if so, which one. I went with GL_OUT_OF_MEMORY. Let me know if that's wrong.

This change is not sufficient to make eglGetError report EGL_CONTEXT_LOST. For a program doing exclusively offscreen rendering, ReadPixels might be the only place where D3DERR_DEVICELOST can be detected. The GLES code can't call egl::setCurrentError though. I'm not sure how to fix that.

[apatrick1, 2010-07-16 19:56:06]

Hi Daniel, I think I found a bug in ReadPixels. Can you review? Thanks.

Al

[dgkoch, 2010-07-21 03:41:30]

Hi Al,

Good catch.  I think the OOM error is really our only suitable error to report
from glReadPixels in this case.  So your patch is fine from that point of view.

Strictly speaking the EGL spec says that only a few functions can generate
EGL_CONTEXT_LOST.  Thus we can't really let a GL function trigger an EGL error. 
If the application simply calls eglMakeCurrent before calling eglGetError this
will report any EGL_CONTEXT_LOST errors without requiring any change to the
current code.

However, if we want to interpret the spec a little more loosely and you do want
to propagate the error back to EGL and get it to notice the lost device so the
next eglGetError call will report it immediately, I would suggest adding a
display->validate() method on the EGLDisplay.   This method could call
device->TestCooperativeLevel() which will report if the d3d device has been lost
and set the EGL_CONTEXT_LOST error if so.  I would be fine with this if you
wanted to add a patch to do so.

Thanks,
Daniel

On 2010-07-16, at 3:56 PM, apatrick@chromium.org wrote:

> Reviewers: dgkoch,
> 
> Message:
> Hi Daniel, I think I found a bug in ReadPixels. Can you review? Thanks.
> 
> Al
> 
> Description:
> ReadPixels recovers from device lost error.
> 
> I found that when I switch desktops, GetRenderTargetData will return
> D3DERR_DEVICELOST. It is unclear to me whether ReadPixels should report
> a GL error in this case and if so, which one. I went with
> GL_OUT_OF_MEMORY. Let me know if that's wrong.
> 
> This change is not sufficient to make eglGetError report
> EGL_CONTEXT_LOST. For a program doing exclusively offscreen rendering,
> ReadPixels might be the only place where D3DERR_DEVICELOST can be
> detected. The GLES code can't call egl::setCurrentError though. I'm not
> sure how to fix that.
> 
> 
> 
> 
> 
> Please review this at http://codereview.appspot.com/1692053/show
> 
> Affected files:
>  M     src/libGLESv2/Context.cpp
> 
> 
> Index: src/libGLESv2/Context.cpp
> ===================================================================
> --- src/libGLESv2/Context.cpp	(revision 342)
> +++ src/libGLESv2/Context.cpp	(working copy)
> @@ -2116,19 +2116,19 @@
> 
>     result = device->GetRenderTargetData(renderTarget, systemSurface);
> 
> -    if (result == D3DERR_DRIVERINTERNALERROR)
> -    {
> -        systemSurface->Release();
> -
> -        return error(GL_OUT_OF_MEMORY);
> -    }
> -
>     if (FAILED(result))
>     {
> -        UNREACHABLE();
>         systemSurface->Release();
> 
> -        return;   // No sensible error to generate
> +        switch (result)
> +        {
> +            case D3DERR_DRIVERINTERNALERROR:
> +            case D3DERR_DEVICELOST:
> +                return error(GL_OUT_OF_MEMORY);
> +            default:
> +                UNREACHABLE();
> +                return;   // No sensible error to generate
> +        }
>     }
> 
>     D3DLOCKED_RECT lock;
> 
> 

---
                        Daniel Koch -+- daniel@transgaming.com
Senior Graphics Architect -+- TransGaming Inc.  -+- www.transgaming.com