Bug 41954 – [Qt] GraphicsLayerQt must have syncCompositingStateForThisLayerOnly() implemented

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jesus Sanchez-Palencia]

[Qt] GraphicsLayerQt must have syncCompositingStateForThisLayerOnly() implemented

[Jesus Sanchez-Palencia]

Created attachment 61036 [details]
Patch

[Jesus Sanchez-Palencia]

Noam and Kenneth, could you please take at this? We need to implement this in order to get AC working with WebKit2.

Thanks in advance!

[Noam Rosenthal]

the first think I'd do for that is to make sure only flushChanges calls m_layer and m_pendingContent, and everyone else calls m_state and m_currentContent.
 I think maybe updateTransform still calls m_layer directly - also the animations, but that's fine because animations work on an already synced layer.

~ m_layer and m_pendingContent contain the non-synced state, m_state and m_currentContent contain the synced state.

[WebKit Commit Bot]

Comment on attachment 61036 [details]
Patch

Clearing flags on attachment: 61036

Committed r63001: <http://trac.webkit.org/changeset/63001>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Jesus Sanchez-Palencia]

Created attachment 61410 [details]
Patch

[Jesus Sanchez-Palencia]

Comment on attachment 61410 [details]
Patch

I'm not going to ask for review it.

No'am, could you please take a look at this follow-up patch? I'm addressing your last comments with a few exceptions.

I noticed that modifying GraphicsLayerQtImpl::recache(), 
GraphicsLayerQtImpl::notifySyncRequired() and GraphicsLayerQt::setContentsToImage() leaves Accelerated Compositing with a strange behavior, crashing often and not working on some examples (i.e.g.: falling leaves).

It would help to have more info on whether a function should use the synced state or the non-synced one. Maybe I missed something...

thanks in advance!

[Noam Rosenthal]

Comment on attachment 61410 [details]
Patch

> -    if (!m_transformAnimationRunning)
> -        m_baseTransform = m_layer->transform();
> +    if (!m_transformAnimationRunning && m_state.maskLayer)
> +        m_baseTransform = m_state.maskLayer->transform();

should be m_state.transform, not m_state.maskLayer->transform. There's no mask layer involved

>  void GraphicsLayerQt::setNeedsDisplay()
>  {
> -    m_impl->m_pendingContent.regionToUpdate = QRegion(QRect(QPoint(0, 0), QSize(size().width(), size().height())));
> +    m_impl->m_currentContent.regionToUpdate = QRegion(QRect(QPoint(0, 0), QSize(size().width(), size().height())));
>      m_impl->notifyChange(GraphicsLayerQtImpl::DisplayChange);
>  }

No need for all that :) it needs to update m_pendingContent and not m_currentContent. The only function we really need to change is updateTransform(), to not read m_layer->transform().

[Jesus Sanchez-Palencia]

Created attachment 61422 [details]
Patch

[Noam Rosenthal]

LGTM