[webkit-reviews] review granted: [Bug 24864] Tidy up some accelerated compositing code related to direct image compositing : [Attachment 28995] Patch, changelog

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Mar 26 18:03:31 PDT 2009


Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 24864: Tidy up some accelerated compositing code related to direct image
compositing
https://bugs.webkit.org/show_bug.cgi?id=24864

Attachment 28995: Patch, changelog
https://bugs.webkit.org/attachment.cgi?id=28995&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> -// A layer can use an inner content layer if the render layer's object is a
replaced object and has no children.
> +// A layer can use direct composoting if the render layer's object is a
replaced object and has no children.

Typo here.

> +void RenderLayerBacking::updateImageContents()
> +{
> +    ASSERT(renderer()->isImage());
> +    RenderImage* imageRenderer = static_cast<RenderImage*>(renderer());
> +    if (imageRenderer->cachedImage() &&
> +	   imageRenderer->cachedImage()->image() &&
> +	   imageRenderer->cachedImage()->isLoaded()) {
> +	   // We have to wait until the image is fully loaded before setting it
on the layer.
> +	   
> +	   // This is a no-op if the layer doesn't have an inner layer for the
image.
> +	  
m_graphicsLayer->setContentsToImage(imageRenderer->cachedImage()->image());
> +	   
> +	   // Image animation is "lazy", in that it automatically stops unless
someone is drawing
> +	   // the image. So we have to kick the animation each time; this has
the downside that the
> +	   // image will keep animating, even if its layer is not visible.
> +	   imageRenderer->cachedImage()->image()->startAnimation();
>      }
>  }

Now that this is in its own function, you can use early returns to get rid of
that cascaded &&, make space to comment about each return if you like, and even
put things into local variables instead of repeating
imageRenderer->cachedImage()->image(). Look how attractive it is!

    CachedImage* cachedImage = imageRenderer->cachedImage();
    if (!cachedImage)
	return;

    Image* image = cachedImage->image();
    if (!image)
	return;

    // We have to wait until the image is fully loaded before setting it on the
layer.
    if (!cachedImage->isLoaded())
	return;

    // This is a no-op if the layer doesn't have an inner layer for the image.
    m_graphicsLayer->setContentsToImage(image);

    // Image animation is "lazy", in that it automatically stops unless someone
is drawing
    // the image. So we have to kick the animation each time; this has the
downside that the
    // image will keep animating, even if its layer is not visible.
    image)->startAnimation();

r=me


More information about the webkit-reviews mailing list