Bug 39753 – ImageSkia doesn't apply antialiasing to rotated image

• Component: Platform
• Status: RESOLVED FIXED

[NovakP]

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?

[NovakP]

Created attachment 57971 [details]
proposed patch

[WebKit Review Bot]

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.

[NovakP]

Created attachment 57972 [details]
Proposed patch, correct style

[David Levin]

Stephen, please look this over.

[Stephen White]

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.)

[David Levin]

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.

[NovakP]

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?

[Stephen White]

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.

[NovakP]

(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.

[Charles Pritchard]

(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.

[NovakP]

(In reply to comment #10)

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