Bug 104535

Created attachment 178995 [details] WIP This fixes the bug. There is not flickering while resizing though especially when trying to make the window smaller.

Comment on attachment 178995 [details] WIP View in context: https://bugs.webkit.org/attachment.cgi?id=178995&action=review > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:227 > ::glFlush(); // Make sure all GL calls have been committed before resizing. Are you sure that EFL does not need to flush here? > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:326 > +#if (!PLATFORM(EFL)) I think we could unify this whole hunk for Qt and EFL. > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:332 > + else if (m_canvasSize != platformLayer->platformLayerSize()) { I am not convinced this is completely correct. The canvas could theoretically change (different token) but have the same size, couldn't it? On the other hand it can never change size, but keep the same token. Or am i missing something the EFL port does? So may be we should check for both? > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:348 > + m_canvasSize = m_canvasPlatformLayer ? m_canvasPlatformLayer->platformLayerSize() : IntSize(); Same here. If you update the size, don't you need to update the token as well?

(In reply to comment #2) > (From update of attachment 178995 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=178995&action=review > > > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:227 > > ::glFlush(); // Make sure all GL calls have been committed before resizing. > > Are you sure that EFL does not need to flush here? > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:326 > > +#if (!PLATFORM(EFL)) > > I think we could unify this whole hunk for Qt and EFL. > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:332 > > + else if (m_canvasSize != platformLayer->platformLayerSize()) { > > I am not convinced this is completely correct. The canvas could theoretically change (different token) but have the same size, couldn't it? > On the other hand it can never change size, but keep the same token. Or am i missing something the EFL port does? > So may be we should check for both? > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:348 > > + m_canvasSize = m_canvasPlatformLayer ? m_canvasPlatformLayer->platformLayerSize() : IntSize(); > > Same here. If you update the size, don't you need to update the token as well? One main difference is that in EFL port, we dont actually recreate the surface for every resize( or delete any associated GL resources). We only resize the window. On Efl port Token never changes. I will remove the ifdefs in the coordinatedgraphicslayer

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 178995 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=178995&action=review > > > > > Source/WebCore/platform/graphics/opengl/GraphicsContext3DOpenGLCommon.cpp:227 > > > ::glFlush(); // Make sure all GL calls have been committed before resizing. > > > > Are you sure that EFL does not need to flush here? > > > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:326 > > > +#if (!PLATFORM(EFL)) > > > > I think we could unify this whole hunk for Qt and EFL. > > > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:332 > > > + else if (m_canvasSize != platformLayer->platformLayerSize()) { > > > > I am not convinced this is completely correct. The canvas could theoretically change (different token) but have the same size, couldn't it? > > On the other hand it can never change size, but keep the same token. Or am i missing something the EFL port does? > > So may be we should check for both? > > > > > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.cpp:348 > > > + m_canvasSize = m_canvasPlatformLayer ? m_canvasPlatformLayer->platformLayerSize() : IntSize(); > > > > Same here. If you update the size, don't you need to update the token as well? > > One main difference is that in EFL port, we dont actually recreate the surface for every resize( or delete any associated GL resources). We only resize the window. > On Efl port Token never changes. I will remove the ifdefs in the coordinatedgraphicslayer The GraphicsSurface API is the lowest common denominator between different platforms. On Mac OS X re-sizing requires re-creating the underlying surface and therefore triggers a token change. As the lowest common denominator that behaviour has become part of the GraphicsSurface API contract, even if some platforms don't actually _do_ require a token change / surface re-creation. Therefore I think we should write code that _uses_ GraphicsSurface based on the API assumptions that hold for all platforms, given that resizing the GL canvas is a case that I would qualify as happening rarely and not worth the optimization, compared to the benefit of less #ifdefs in WebKit code and more code sharing across ports.

Created attachment 179031 [details] patch

Comment on attachment 179031 [details] patch Clearing flags on attachment: 179031 Committed r137467: <http://trac.webkit.org/changeset/137467>

All reviewed patches have been landed. Closing bug.