Bug 93040 – [BlackBerry] Upstream GraphicsLayerClient::contentsVisible()

• Component: WebKit BlackBerry
• Status: RESOLVED FIXED

[Arvid Nilsson]

Our LayerTiler uses both proactive and reactive rendering to populate tiles. If contentsVisible() is accurate, it will cause the right tiles to be rendered. Failing that, when a dirty tile is found to be visible on the compositing thread, a render job is scheduled.

[Arvid Nilsson]

Created attachment 156199 [details]
Patch

[WebKit Review Bot]

Comment on attachment 156199 [details]
Patch

Clearing flags on attachment: 156199

Committed r124544: <http://trac.webkit.org/changeset/124544>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Antonio Gomes]

Comment on attachment 156199 [details]
Patch

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

Post-land comments:

> Source/WebCore/platform/graphics/GraphicsLayerClient.h:89
> +    virtual bool contentsVisible(const GraphicsLayer*, const IntRect& contentRect) const = 0;

but it would be preferable to add a default blank implementation here, instead of a pure virtual method, and get rid of the #if platform(bb)?

> Source/WebCore/rendering/RenderLayerBacking.cpp:1632
> +    IntRect visibleContentRect(view->visibleContentRect());
> +    FloatQuad absoluteContentQuad = renderer()->localToAbsoluteQuad(FloatRect(localContentRect));
> +    return absoluteContentQuad.enclosingBoundingBox().intersects(visibleContentRect);

does this work for subframes?

[Arvid Nilsson]

Gnnh, I forgot to add the declaration to RenderLayerBacking.h

[Arvid Nilsson]

Comment on attachment 156199 [details]
Patch

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

>> Source/WebCore/platform/graphics/GraphicsLayerClient.h:89
>> +    virtual bool contentsVisible(const GraphicsLayer*, const IntRect& contentRect) const = 0;
> 
> but it would be preferable to add a default blank implementation here, instead of a pure virtual method, and get rid of the #if platform(bb)?

There's a default implementation coming in https://bugs.webkit.org/show_bug.cgi?id=93036, however it's still not enabling it for all platforms.

>> Source/WebCore/rendering/RenderLayerBacking.cpp:1632
>> +    return absoluteContentQuad.enclosingBoundingBox().intersects(visibleContentRect);
> 
> does this work for subframes?

I think this is actually returns true too often, which is better than too seldom - if my thinking is correct, this will return true if the layer is visible in the subframe's viewport, even if the whole viewport is scrolled off screen.

[Antonio Gomes]

> > does this work for subframes?
> 
> I think this is actually returns true too often, which is better than too seldom - if my thinking is correct, this will return true if the layer is visible in the subframe's viewport, even if the whole viewport is scrolled off screen.

I do not think it clips against its parent viewport.

[Arvid Nilsson]

(In reply to comment #7)
> > > does this work for subframes?
> > 
> > I think this is actually returns true too often, which is better than too seldom - if my thinking is correct, this will return true if the layer is visible in the subframe's viewport, even if the whole viewport is scrolled off screen.
> 
> I do not think it clips against its parent viewport.

Exactly, I guess that was what I was trying to say. So it will return true too often (false positive) but that's ok considering the use case - when to populate tiles. ,maybe there is benefit from making it more accurate, though.