[Webkit-unassigned] [Bug 88908] [chromium] webkit-backface-visibility doesn't work with <video>

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=88908





--- Comment #5 from Shawn Singh <shawnsingh at chromium.org>  2012-06-12 16:25:42 PST ---
(From update of attachment 147178)

In addition to comments below, let's add a layout test that reproduces the problem and gets fixed by this patch.  You can probably do this by assimilating the test case that we were given originally, and making it in the style of a webkit layout test.

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

> Source/Platform/ChangeLog:15
> +        * chromium/public/WebContentLayer.h:
> +        (WebContentLayer):
> +        * chromium/public/WebLayer.h:
> +        (WebLayer):

I wonder if its possible to take different approach that keeps this fix contained to graphicsLayerChromium?  I'm not sure it would work, but we should try the following:
  - find a solution that doesn't require setIsContentsLayer
  - don't remove setIsDoubleSided from WebContentsLayer
  - instead, add the complexity to GraphicsLayerChromium::setBackfaceVisibility().

> Source/Platform/chromium/public/WebContentLayer.h:-58
> -    WEBKIT_EXPORT void setDoubleSided(bool);

I'm guessing we shouldn't remove this.  Did you remove it thinking that this corresponded to GraphicsLayerChromium::m_contentsLayer?   looking at the code, its actually backwards - GraphicsLayerChromium:m_layer is actually a WebContentsLayer, and GraphicsLayerChromium::m_contentsLayer is actually a WebLayer.  (!!!!)   I'm not sure if the naming should change or not, we should ask jamesr about that.   All the more reason to keep the fix within GraphicsLayerChromium if that works.

> Source/WebCore/platform/graphics/chromium/GraphicsLayerChromium.cpp:822
> +        m_contentsLayer.setDoubleSided(m_layer.doubleSided());

In addition to placing it here, I think adding this logic to GraphicsLayerChromium::setBackfaceVisibility might be the solution we're looking for.   Note however, I don't think its as simple as setting it to m_layer.doubleSided().  You can see if it passes all layout tests, and if it doesnt, we'll probably have to visit the W3C spec to understand how to set m_contentsLayer.setDoubleSided(...) correctly.

> Source/WebCore/platform/graphics/chromium/LayerChromium.h:346
> +    bool m_isContentsLayer;

isContentsLayer might be a bit vague by itself.  Hopefully we won't even need this flag, if GraphicsLayerChromium can do the correct W3C logic on its own to decide how to set doubleSided.  but if we need this, perhaps we should rename it to "m_isContentsLayerOfGraphicsLayer"

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.