[webkit-reviews] review granted: [Bug 102501] [TexMap] Flickering after transitions on Apple HTML5 demo : [Attachment 184721] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has granted Alexander Paschenko
<alexander.pashenko at lge.com>'s request for review:
Bug 102501: [TexMap] Flickering after transitions on Apple HTML5 demo
https://bugs.webkit.org/show_bug.cgi?id=102501

Attachment 184721: Patch
https://bugs.webkit.org/attachment.cgi?id=184721&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184721&action=review


Great! Please update the changelog...

> Source/WebCore/ChangeLog:22
> +	   This patch solves the problem by introducing two additional private
flags to TextureMapperLayer: m_shouldUpdateCurrentTransformFromGraphicsLayer
and m_shouldUpdateCurrentOpacityFromGraphicsLayer.
> +	   On these flags' basis, TextureMapperLayer is able to decide whether
to update its inner state or not.
> +	   These flags themselves are set based on GraphicsLayerTextureMapper's
changeMask which indicates what details of the state have been changed since
the last sync.
> +	   Also, to avoid similar problems with m_currentFilters in the future,
a flag with similar meaning named m_shouldUpdateCurrentFiltersFromGraphicsLayer
has been introduced. Corresponding methods have been updated accordingly.
> +
> +	   No new tests - this doesn't expose any testable surface. Eyes-only
check has been made to ensure that the problem is gone now.
> +
> +	   * platform/graphics/texmap/TextureMapperLayer.cpp:
> +	   (WebCore::TextureMapperLayer::setAnimatedTransform): sets
m_shouldUpdateCurrentTransformFromGraphicsLayer to false and updates
m_currentTransform based on the updated state from GraphicsLayerAnimation.
> +	   (WebCore):
> +	   (WebCore::TextureMapperLayer::setAnimatedOpacity): sets
m_shouldUpdateCurrentOpacityFromGraphicsLayer to false and updates
m_currentOpacity based on the updated state from GraphicsLayerAnimation.
> +	   (WebCore::TextureMapperLayer::setAnimatedFilters): sets
m_shouldUpdateCurrentFiltersFromGraphicsLayer to false and updates
m_currentFilters based on the updated state from GraphicsLayerAnimation.
> +	  
(WebCore::TextureMapperLayer::flushCompositingStateForThisLayerOnly): sets
m_shouldUpdateCurrent* flags based on GLTM's changeMask. Also illegal
modification of m_currentTransform that caused flickering has been removed from
this method.
> +	   (WebCore::TextureMapperLayer::syncAnimations): updates
m_currentTransform and/or m_currentOpacity and/or m_currentFilters if
corresponding flags allow to do so.

Please keep Changelog lines at max of 100.
Also, you say "two" additional flags, where we have 3 :)