[Webkit-unassigned] [Bug 45476] [Chromium] Minimize uploads in canvas 2d mixed mode rendering

[bugzilla-daemon at webkit.org]

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


James Robinson <jamesr at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #67070|review?                     |review-
               Flag|                            |




--- Comment #3 from James Robinson <jamesr at chromium.org>  2010-09-09 12:12:40 PST ---
(From update of attachment 67070)
review- for the build break.  Overall looks awesome.


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

> WebCore/html/canvas/CanvasRenderingContext2D.cpp:1534
>  #if ENABLE(ACCELERATED_2D_CANVAS) && USE(ACCELERATED_COMPOSITING)
> +    if (isAccelerated())
> +        drawingContext()->addDirtyRect(enclosingIntRect(dirtyRect));
The addDirtyRect() call should only be guarded by ENABLE(ACCELERATED_2D_CANVAS).  We still have to make this call even if accelerated compositing is not enabled in order to render correctly.

> WebCore/html/canvas/CanvasRenderingContext2D.h:272
> +        CanvasDidDrawApplyNone = 0
nit: this would be better at the beginning of the enum definition

> WebCore/platform/graphics/GraphicsContext.h:427
> +        void addDirtyRect(const IntRect& rect);
You have to add a stub implementation of this to GraphicsContext.cpp for !PLATFORM(SKIA) or this won't compile on other platforms.  See how setSharedGraphicsContext3D and syncSoftwareCanvas() are handled.

Don't name the parameter here.

Also, this name is less than ideal.  It appears to have a much broader application than it really does.

> WebCore/platform/graphics/gpu/Texture.cpp:147
> +
> +
remove the extra new lines here

> WebCore/platform/graphics/skia/PlatformContextSkia.cpp:818
> +    switch (m_backingStoreState) {
> +    case Software:
> +    case Mixed:
> +        m_softwareDirtyRect.unite(rect);
> +        return;
> +    case Hardware:
> +        return;
> +    default:
> +        ASSERT_NOT_REACHED();
This is probably clearer as an if() instead of a switch.  The other cases are not likely to get interesting.

> WebCore/platform/graphics/skia/PlatformContextSkia.h:197
> +    void addDirtyRect(const IntRect& rect);
don't name the parameter here

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