Bug 103297 – Coordinated Graphics: Refactor code related to mask and replication.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dongseong Hwang]

This patch makes code related to mask and replication in
GraphicsLayerTextureMapper be consistent with them in CoordinatedGraphicsLayer.

1. setSize().
GraphicsLayerTextureMapper::setSize() also sets the size of a mask layer like
CoordinatedGraphicsLayer. setMaksLayer ditto.

2. setContentsVisible().
GraphicsLayerTextureMapper::setContentsVisible() and
CoordinatedGraphicsLayer::setContentsVisible() set the visibility of a mask layer
like setSize().

3. recursive traverse.
We sometime miss traversing a mask or replication layer. This patch fixs them.

4. remove maskTarget in CoordinatedGraphicsLayer.
It is not used.

[Dongseong Hwang]

Created attachment 176071 [details]
Patch

[Noam Rosenthal]

Comment on attachment 176071 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=176071&action=review

Those fixes are different, and should come in separate patches. You don't need new bug reports, simply attach different patches to the same bug.

> Source/WebKit2/ChangeLog:24
> +        4. remove maskTarget in CoordinatedGraphicsLayer.
> +        It is not used.

This is the only part that should apply to this changelog.

[Dongseong Hwang]

Created attachment 176141 [details]
Patch

[Dongseong Hwang]

Created attachment 176145 [details]
Patch. Coordinated Graphics: traverse a mask layer and a replication layer too.

[Dongseong Hwang]

Created attachment 176146 [details]
Patch4

[Dongseong Hwang]

Created attachment 176150 [details]
Patch

[Dongseong Hwang]

(In reply to comment #2)
> (From update of attachment 176071 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176071&action=review
> 
> Those fixes are different, and should come in separate patches. You don't need new bug reports, simply attach different patches to the same bug.
> 
> > Source/WebKit2/ChangeLog:24
> > +        4. remove maskTarget in CoordinatedGraphicsLayer.
> > +        It is not used.
> 
> This is the only part that should apply to this changelog.

Thanks for review!

I separated 4 patches.

[WebKit Review Bot]

Comment on attachment 176141 [details]
Patch

Clearing flags on attachment: 176141

Committed r135814: <http://trac.webkit.org/changeset/135814>

[Noam Rosenthal]

Comment on attachment 176145 [details]
Patch. Coordinated Graphics: traverse a mask layer and a replication layer too.

View in context: https://bugs.webkit.org/attachment.cgi?id=176145&action=review

We shouldn't traverse mask and replica layers in this way. 
Backing store for masks depend on their owning layer, and replica layers don't have a backing store. We should make more conscious decisions about this rather than traverse everything.

> Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:57
> +    if (m_state.replicaLayer)
> +        m_state.replicaLayer->clearBackingStoresRecursive();

A replica layer never has a backing store.

[Noam Rosenthal]

Comment on attachment 176146 [details]
Patch4

View in context: https://bugs.webkit.org/attachment.cgi?id=176146&action=review

> Source/WebKit2/ChangeLog:8
> +        The member is not used.

Remove the maskTarget member of CoordinatedGraphicsLayer, which is not used.

[Dongseong Hwang]

(In reply to comment #9)
> (From update of attachment 176145 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176145&action=review
> 
> We shouldn't traverse mask and replica layers in this way. 
> Backing store for masks depend on their owning layer, and replica layers don't have a backing store. We should make more conscious decisions about this rather than traverse everything.
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperLayer.cpp:57
> > +    if (m_state.replicaLayer)
> > +        m_state.replicaLayer->clearBackingStoresRecursive();
> 
> A replica layer never has a backing store.

Thanks for explanation. My lack of understanding made this patch.
The part of Coordinated Graphics is also useless, right?

[Dongseong Hwang]

Created attachment 176168 [details]
Patch

[Dongseong Hwang]

(In reply to comment #10)
> (From update of attachment 176146 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=176146&action=review
> 
> > Source/WebKit2/ChangeLog:8
> > +        The member is not used.
> 
> Remove the maskTarget member of CoordinatedGraphicsLayer, which is not used.

Thanks for rephrasing. It is done in following patch.

(In reply to comment #12)
> Created an attachment (id=176168) [details]
> Patch

[Dongseong Hwang]

Created attachment 176172 [details]
Patch7 that changed changelog of Patch4

[WebKit Review Bot]

Comment on attachment 176150 [details]
Patch

Clearing flags on attachment: 176150

Committed r135818: <http://trac.webkit.org/changeset/135818>

[WebKit Review Bot]

Comment on attachment 176172 [details]
Patch7 that changed changelog of Patch4

Clearing flags on attachment: 176172

Committed r135830: <http://trac.webkit.org/changeset/135830>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.