[Webkit-unassigned] [Bug 37069] [chromium] DragImage implementation for mac

[bugzilla-daemon at webkit.org]

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


Avi Drissman <avi at drissman.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |avi at drissman.com




--- Comment #9 from Avi Drissman <avi at drissman.com>  2010-04-06 14:25:53 PST ---
Not authorized to actually comment on the patch directly; here are some
thoughts:

> Index: WebCore/platform/chromium/DragImageChromiumMac.cpp
> ===================================================================
> +#include <CoreGraphics/CGBitmapContext.h>
> +#include <CoreGraphics/CGImage.h>

Including headers of a sub-framework is discouraged. Try
<ApplicationServices/ApplicationServices.h>.

>  DragImageRef scaleDragImage(DragImageRef image, FloatSize scale)
>  {
...
> +    size_t width = roundf(CGImageGetWidth(image) * scale.width());
> +    size_t height = roundf(CGImageGetHeight(image) * scale.height());
> +    CGContextRef context = CGBitmapContextCreate(0, width, height, 8, width * 4, CGImageGetColorSpace(image), kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);

We usually only need to specify flags or byte ordering if we're hammering at
the bitmap that we're creating. Since we're going straight to a CGImage, we
probably don't need to specify anything special here. (And ditto for several
similar uses below.)

> +    if (!context)
> +        return 0;
> +    CGContextDrawImage(context, CGRectMake(0, 0, width, height), image);
> +    CGImageRelease(image);

I'm not familiar with the ownership context. Is the caller relinquishing
ownership in a call named scaleDragImage? (And ditto for several similar uses
below.)

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