[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 #6 from James Robinson <jamesr at chromium.org>  2012-06-12 17:30:20 PST ---
(In reply to comment #5)
> (From update of attachment 147178 [details])
> 
> 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().

I like this idea a lot.

While you're doing that, could you construct test cases for other content layer types (canvas 2d, webgl, etc) and see if they suffer from the same issue?

> 
> > 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.

Terminology inversion here.  In GraphicsLayer terms, m_layer = the layer representing the GraphicsLayerClient's painted stuff.  m_contentsLayer = the special stuff inside the layer (video, webgl, whatever).

In the WebLayer or LayerChromium hierarchy, we use a different terminology.  *ContentLayer* means a layer type that draws via a paintContents() call.  WebLayer/LayerChromium are the base class representing any time of layer.  Thus, GraphicsLayer::m_layer is a *ContentLayer* layer because it renders with a paint() call.  GraphicLayer::m_contentLayer is just of the base class type since we don't know which concrete type it is (WebGL, video, whatever).

I agree it sucks but I'm not completely sure what to do about it.  We could rename GraphicsLayerChromium's members, but we want them to be somewhat consistent with the cross-platform GraphicsLayer interface.

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