Bug 83691 - [Texmap] Falling leaves demo missing opacity fade out animation
Summary: [Texmap] Falling leaves demo missing opacity fade out animation
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Noam Rosenthal
URL: http://www.webkit.org/blog-files/leaves/
Depends on:
Reported: 2012-04-11 08:17 PDT by Lars Knudsen
Modified: 2012-04-28 10:26 PDT (History)
4 users (show)

See Also:

Very compact version of falling leaves exposing the problem (1.30 KB, text/html)
2012-04-19 06:19 PDT, Lars Knudsen
no flags Details
Patch (13.64 KB, patch)
2012-04-27 13:34 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (13.98 KB, patch)
2012-04-27 15:05 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff
Patch (14.28 KB, patch)
2012-04-27 15:26 PDT, Noam Rosenthal
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Knudsen 2012-04-11 08:17:47 PDT
Looking at 


with the MiniBrowser (Qt, WK2), the leaves are missing the opacity fade out animation, making them disappear abruptly.
Comment 1 Igor Trindade Oliveira 2012-04-11 09:55:53 PDT
GTK has the same behavior.
Comment 2 Lars Knudsen 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.
Comment 3 Noam Rosenthal 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.
Comment 4 Noam Rosenthal 2012-04-27 13:34:49 PDT
Created attachment 139261 [details]
Comment 5 Martin Robinson 2012-04-27 14:09:55 PDT
Comment on attachment 139261 [details]

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.
Comment 6 Noam Rosenthal 2012-04-27 15:05:52 PDT
Created attachment 139285 [details]
Comment 7 Noam Rosenthal 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.
Comment 8 Noam Rosenthal 2012-04-27 15:26:01 PDT
Created attachment 139289 [details]
Comment 9 Martin Robinson 2012-04-28 09:34:49 PDT
Comment on attachment 139289 [details]

Comment 10 WebKit Review Bot 2012-04-28 10:26:16 PDT
Comment on attachment 139289 [details]

Clearing flags on attachment: 139289

Committed r115574: <http://trac.webkit.org/changeset/115574>
Comment 11 WebKit Review Bot 2012-04-28 10:26:20 PDT
All reviewed patches have been landed.  Closing bug.