WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
104535
[EFL [WebGL] [Wk2] Resizing the canvas breaks WebGL
https://bugs.webkit.org/show_bug.cgi?id=104535
Summary
[EFL [WebGL] [Wk2] Resizing the canvas breaks WebGL
Kalyan
Reported
2012-12-10 04:19:10 PST
Try
https://www.khronos.org/registry/webgl/sdk/demos/webkit/SpiritBox.html
in MiniBrowser. Resize the view. Expected Result: View is resized properly with blue background and the cube is spinning. Actual Result: View is resized but we dont see any blue background or spinning cube.
Attachments
WIP
(5.59 KB, patch)
2012-12-12 01:19 PST
,
Kalyan
no flags
Details
Formatted Diff
Diff
patch
(7.04 KB, patch)
2012-12-12 07:02 PST
,
Kalyan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Kalyan
Comment 1
2012-12-12 01:19:21 PST
Created
attachment 178995
[details]
WIP This fixes the bug. There is not flickering while resizing though especially when trying to make the window smaller.
Zeno Albisser
Comment 2
2012-12-12 01:38:00 PST
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?
Kalyan
Comment 3
2012-12-12 01:49:44 PST
(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
Simon Hausmann
Comment 4
2012-12-12 04:02:43 PST
(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.
Kalyan
Comment 5
2012-12-12 07:02:37 PST
Created
attachment 179031
[details]
patch
WebKit Review Bot
Comment 6
2012-12-12 07:56:47 PST
Comment on
attachment 179031
[details]
patch Clearing flags on attachment: 179031 Committed
r137467
: <
http://trac.webkit.org/changeset/137467
>
WebKit Review Bot
Comment 7
2012-12-12 07:56:52 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug