|Summary:||[chromium] Check getGraphicsResetStatusARB and reinitialize the renderer in an error is returned.|
|Product:||WebKit||Reporter:||Alexey Marinichev <amarinichev>|
|Component:||WebCore Misc.||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||commit-queue, enne, kbr, vangelis|
|Version:||528+ (Nightly build)|
Description Alexey Marinichev 2010-10-18 14:18:45 PDT
This is a step towards proper handling of lost context events.
Comment 2 Vangelis Kokkevis 2010-10-18 16:33:06 PDT
Comment on attachment 71082 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71082&action=review > WebKit/chromium/src/WebViewImpl.cpp:2484 > +void WebViewImpl::reallocateRenderer(void) I don't believe that (void) is common in WebKit for methods that take no arguments. > WebKit/chromium/src/WebViewImpl.cpp:2487 > + RefPtr<GraphicsContext3D> newContext = GraphicsContext3D::create(context->getContextAttributes(), m_page->chrome(), GraphicsContext3D::RenderDirectlyToHostWindow); It's not guaranteed that either the newContext or the layerRenderer will be non-null. We need to check against failures. There's some logic to keep track of failed creation attempts in WebViewImpl::setIsAcceleratedCompositingActive() that we'll need to use here as well. If the context is lost, what are the chances we'll be able to re-create it right away? If we fail to create a context should we give up after the first attempt? We'll need to figure out what happens in a real context lost event on windows. > WebKit/chromium/src/WebViewImpl.cpp:2494 > + m_layerRenderer->rootLayer()->setNeedsDisplayRecursive(); I think that instead of calling setNeedsDisplayRecursive, you can modify LayerChromium::setLayerRenderer to call setNeedsDisplay() after calling cleanupResources(). That way, even layers that are currently not connected to layer hierarchy for some reason will be marked dirty before trying to render.
Comment 3 Alexey Marinichev 2010-10-20 18:07:16 PDT
Created attachment 71376 [details] addressed comments
Comment 4 Kenneth Russell 2010-10-25 19:37:31 PDT
Comment on attachment 71376 [details] addressed comments View in context: https://bugs.webkit.org/attachment.cgi?id=71376&action=review This looks good. Couple of tiny comments, not necessary to address (marking cq+). > WebCore/ChangeLog:6 > + renderer in an error is returned. in -> if > WebKit/chromium/ChangeLog:6 > + renderer in an error is returned. in -> if > WebKit/chromium/src/WebViewImpl.cpp:2489 > + RefPtr<LayerRendererChromium> layerRenderer = LayerRendererChromium::create(newContext); Strictly speaking this should probably be newContext.release().
Comment 5 WebKit Commit Bot 2010-10-26 11:18:58 PDT
Comment on attachment 71376 [details] addressed comments Clearing flags on attachment: 71376 Committed r70543: <http://trac.webkit.org/changeset/70543>
Comment 6 WebKit Commit Bot 2010-10-26 11:19:03 PDT
All reviewed patches have been landed. Closing bug.