Bug 39753 - ImageSkia doesn't apply antialiasing to rotated image
Summary: ImageSkia doesn't apply antialiasing to rotated image
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL: http://dl.dropbox.com/u/841468/testca...
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-26 11:12 PDT by NovakP
Modified: 2012-10-08 16:38 PDT (History)
5 users (show)

See Also:


Attachments
Difference without/with setAntiAlias (34.36 KB, image/jpeg)
2010-05-26 11:12 PDT, NovakP
no flags Details
proposed patch (1.09 KB, patch)
2010-06-06 02:50 PDT, NovakP
no flags Details | Formatted Diff | Diff
Proposed patch, correct style (1.11 KB, patch)
2010-06-06 03:23 PDT, NovakP
levin: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description NovakP 2010-05-26 11:12:24 PDT
Created attachment 57117 [details]
Difference without/with setAntiAlias

Chromium's Skia library doesn't apply antialiasing to rotated image, as you can see in the attachment on the left side.

When I added the line
  paint.setAntiAlias(true);
to WebCore\platform\graphics\skia\ImageSkia.cpp to method paintSkBitmap,
and builded Chromium, it draws the image correctly, as you can see in the attachment on the right side.

I don't have toolchain for making WebKit patches, so if this is correct change, would someone make the patch and commit it?
Comment 1 NovakP 2010-06-06 02:50:06 PDT
Created attachment 57971 [details]
proposed patch
Comment 2 WebKit Review Bot 2010-06-06 02:52:06 PDT
Attachment 57971 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/skia/ImageSkia.cpp:243:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 NovakP 2010-06-06 03:23:08 PDT
Created attachment 57972 [details]
Proposed patch, correct style
Comment 4 David Levin 2010-06-14 11:28:38 PDT
Stephen, please look this over.
Comment 5 Stephen White 2010-06-14 11:51:45 PDT
This will probably have some serious performance implications, which is why it's not currently enabled.  If you can show that the performance delta is acceptable, we could consider it.

(There would also be a boatload of pixel tests to re-do, but that's just gruntwork for someone to do.)
Comment 6 David Levin 2010-06-15 00:06:40 PDT
Comment on attachment 57972 [details]
Proposed patch, correct style

r- due to Stephen's comment. Feel free to set back to r? if you can address his (perf) comment.
Comment 7 NovakP 2010-06-16 05:16:36 PDT
I made a testcase with 24 rotated images and measured paint duration with Speed Tracer (Is there some better solution to measure paint perf?)

Build w/o antialias, paint average duration: 17 ms
Build with antialias, paint average duration: 20 ms

Is this acceptable or not?
Comment 8 Stephen White 2010-06-16 10:05:54 PDT
It seems this is the same as http://crbug.com/7508, and there is debate as to whether this is a good idea.  E.g., for <canvas> tags it will cause overlapped textured images to have cracks along the seams.

Of course, those applications are already broken in other browsers, but until there's a strong consensus to change it, or the spec commits one way or the other, I'd prefer to leave it as-is.
Comment 9 NovakP 2010-06-17 08:29:12 PDT
(In reply to comment #8)

I think it would be better to match other browsers behavior in CSS 2D Transforms (Safari, Firefox, Opera already do antialiasing).
Canvas is slightly other case for me, it's "drawing board", you can do things like overlapped images in canvas easily, so it's better to leave images in canvas aliased. My patch doesn't affect canvas rendering, so it isn't same as issue #7508 in Chromium.
Comment 10 Charles Pritchard 2010-11-27 11:52:28 PST
(In reply to comment #9)
> I think it would be better to match other browsers behavior in CSS 2D Transforms (Safari, Firefox, Opera already do antialiasing).

Has this made it in to the main line? From what I've seen, CSS 2D transforms are anti-aliased in Chromium: it looks like this patch/functionality made it in.
Comment 11 NovakP 2010-11-27 12:12:42 PST
(In reply to comment #10)

This bug is about antialiasing transformed images, other objects are already antialiased.