Bug 68295 – Remove preserves3D() from CCLayerDelegate

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

[Antoine Labour]

Remove preserves3D() from CCLayerDelegate

[Antoine Labour]

Created attachment 107755 [details]
Patch

[Antoine Labour]

Created attachment 107757 [details]
Patch

[Antoine Labour]

Created attachment 108436 [details]
Patch

[Antoine Labour]

Rebased patch (and friendly ping)

[James Robinson]

Comment on attachment 108436 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:367
> +        platformLayer->setPreserves3D(m_preserves3D);

I don't think we need to do this - the canvas contents layer can't have preserves3D set since it's a CSS property and would only apply to the container layer, and even if it could we should be able to rely on the normal property propagation code

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:400
> +        layer->setPreserves3D(m_preserves3D);

same here - i don't think media layers can have this bit

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:600
> +    if (m_contentsLayer)
> +        m_contentsLayer->setPreserves3D(m_preserves3D);

it looks like we used to propagate the preserves3D bit to the contents layer.  this change preserves that behavior but I think it's actually wrong. Can you see if any tests break if you only propagate the preserves3D bit to the m_layer?

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:652
> +        m_contentsLayer->setPreserves3D(m_preserves3D);

do we need this?

[James Robinson]

Comment on attachment 108436 [details]
Patch

R=me since this is preserving the existing behavior, so it's fine to land as-is.  I think we can simplify, however.

[Antoine Labour]

Created attachment 108551 [details]
Patch

[Antoine Labour]

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

>> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:367
>> +        platformLayer->setPreserves3D(m_preserves3D);
> 
> I don't think we need to do this - the canvas contents layer can't have preserves3D set since it's a CSS property and would only apply to the container layer, and even if it could we should be able to rely on the normal property propagation code

Done.

>> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:400
>> +        layer->setPreserves3D(m_preserves3D);
> 
> same here - i don't think media layers can have this bit

Done.

>> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:600
>> +        m_contentsLayer->setPreserves3D(m_preserves3D);
> 
> it looks like we used to propagate the preserves3D bit to the contents layer.  this change preserves that behavior but I think it's actually wrong. Can you see if any tests break if you only propagate the preserves3D bit to the m_layer?

Done. Layout tests still pass.

>> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:652
>> +        m_contentsLayer->setPreserves3D(m_preserves3D);
> 
> do we need this?

Removed.

[James Robinson]

Comment on attachment 108551 [details]
Patch

R=me

[WebKit Review Bot]

Comment on attachment 108551 [details]
Patch

Clearing flags on attachment: 108551

Committed r95886: <http://trac.webkit.org/changeset/95886>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.