Bug 83691

Summary: [Texmap] Falling leaves demo missing opacity fade out animation
Product: WebKit Reporter: Lars Knudsen <larsgk>
Component: WebKit2Assignee: Noam Rosenthal <noam>
Status: RESOLVED FIXED    
Severity: Normal CC: igor.oliveira, mrobinson, noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: http://www.webkit.org/blog-files/leaves/
Attachments:
Description Flags
Very compact version of falling leaves exposing the problem
none
Patch
none
Patch
none
Patch none

Lars Knudsen
Reported 2012-04-11 08:17:47 PDT
Looking at http://www.webkit.org/blog-files/leaves/ with the MiniBrowser (Qt, WK2), the leaves are missing the opacity fade out animation, making them disappear abruptly.
Attachments
Very compact version of falling leaves exposing the problem (1.30 KB, text/html)
2012-04-19 06:19 PDT, Lars Knudsen
no flags
Patch (13.64 KB, patch)
2012-04-27 13:34 PDT, Noam Rosenthal
no flags
Patch (13.98 KB, patch)
2012-04-27 15:05 PDT, Noam Rosenthal
no flags
Patch (14.28 KB, patch)
2012-04-27 15:26 PDT, Noam Rosenthal
no flags
Igor Trindade Oliveira
Comment 1 2012-04-11 09:55:53 PDT
GTK has the same behavior.
Lars Knudsen
Comment 2 2012-04-19 06:19:29 PDT
Created attachment 137889 [details] Very compact version of falling leaves exposing the problem It seems that when rendering in layers having opacity, forcing the intermediate surface rendering to take place (in TextureMapperLayer::paintRecursive) - something goes wrong with either the transformation matrix, clipping, gl surface rendering or the like (I am still learning - so it might be obvious to others). The included example shows how only a *part* of the fading rectangle (id = "nodeContainer" in the example) is visible when opacity < 1. Digging further.
Noam Rosenthal
Comment 3 2012-04-27 09:34:55 PDT
I'm pretty sure that the issue comes from the behavior of glClear, that is affected by glScissor.
Noam Rosenthal
Comment 4 2012-04-27 13:34:49 PDT
Martin Robinson
Comment 5 2012-04-27 14:09:55 PDT
Comment on attachment 139261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139261&action=review This looks good to me. I have one concern below. The fact that the clip stack is now duplicated between the BitmapTexture and the shared TextureMapperGL state (framebuffer for the entire scene), suggests to me that perhaps there could be an abstraction to encapsulate each framebuffer and its associated clip stack. There would be an instance for the entire scene and one for each BitmapTexture. > Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp:541 > + m_clipStack.clear(); > + m_clipStack.current().stencilIndex = 1; > + m_clipStack.current().scissorBox = IntRect(IntPoint::zero(), m_textureSize); > + m_clipStack.apply(); > + GL_CMD(glClearColor(0, 0, 0, 0)); > + GL_CMD(glClear(GL_COLOR_BUFFER_BIT)); Will this fail if TextureMapperGL::beginClip was never called from the WebKit layer? That's what enables the scissor test. Now you are removing the call to ::beginClip here and just applying the clip stack directly. It seems like you also need to enable the scissor test here as well. Perhaps that should just happen the first time you need to apply a scissorred clip. You might also need to somehow handle disabling the scissor test when rebinding TextureMapperGL's main framebuffer.
Noam Rosenthal
Comment 6 2012-04-27 15:05:52 PDT
Noam Rosenthal
Comment 7 2012-04-27 15:06:39 PDT
I've simplified things a bit, by making SCISSOR_TEST always be on, and sharing a bit more code inside ClipStack.
Noam Rosenthal
Comment 8 2012-04-27 15:26:01 PDT
Martin Robinson
Comment 9 2012-04-28 09:34:49 PDT
Comment on attachment 139289 [details] Patch Great!
WebKit Review Bot
Comment 10 2012-04-28 10:26:16 PDT
Comment on attachment 139289 [details] Patch Clearing flags on attachment: 139289 Committed r115574: <http://trac.webkit.org/changeset/115574>
WebKit Review Bot
Comment 11 2012-04-28 10:26:20 PDT
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.