[Webkit-unassigned] [Bug 39557] Full screen doesn't work for video elements

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Chris Marrin <cmarrin at apple.com>  2010-05-24 06:54:35 PST ---
(In reply to comment #12)
> (In reply to comment #11)
> > > +    float aspectRatio() const { return m_width / m_height; }
> > 
> > You should zero check m_height here to avoid divide by 0
> 
> Why? If m_height is 0, it will return either +INF or -INF, which is a true answer for the size's aspect ratio.

Are we sure no platform will throw a divide by 0 exception? If so, then it's fine. I'm just not aware of code elsewhere in WebKit that allows these sorts of shenanigans.

...
> > > +    if (m_rootChild) {
> > > +        m_layerRenderer->setRootChildLayer(m_rootChild.get());
> > > +        WKCACFLayer* rootLayer = m_rootChild->rootLayer();
> > > +        CGRect rootBounds = m_rootChild->rootLayer()->bounds();
> > > +        m_rootChild->setFrame(rootBounds);
> > 
> > WKCACFLayer no longer has a setFrame() API. I removed it to simplify the API. Frame is always so confusing because it's nothing more than a composite of bounds.size and position. So you can replace this with calls to setBounds and setPosition.
> 
> In order to reproduce the setFrame() API, the caller would need to get the anchorPoint, multiply the result by the destination bounds width and height, and add the result to the intended bottom-left corner position, and then call setBounds() and setPosition().  This turns one line of code into 7 or 8 lines of code.  Why should we remove an API that just wraps CACFLayerSetFrame()?

We never use this API in GraphicsLayer. We have to do lots of logic to flip Y and do other conversions so we always do that at the higher levels anyway. I had a couple of places where it was done in WKCACFLayer. Because my position is always(0,0), I could set the anchor point to (0,0) on creation and then just return setFrame with setBounds. If your anchor point never changes (and I assume that's the case) you can simply set the anchor point to (0,0) and then call setBounds(CGRectMake(0,0, rootBounds.size.width, rootBounds.size.height); setPosition(rootBounds.origin).

I removed setFrame to avoid needing to deal with its complexities in my tiled layer logic. It's long been the source of confusion and bugs, and since it is used so rarely, I felt (and still do) that avoiding its use is the best approach.

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