Bug 79652

Summary:

Add Conditions of Antialias for Drawing Lines

Product:

WebKit

Reporter:

younghwan cho <yhwan.cho>

Component:

WebCore Misc.

Assignee:

Nobody <webkit-unassigned>

Status:

UNCONFIRMED

Severity:

Normal

CC:

ap, dglazkov, hyatt, hyunki.baik, igor.oliveira, simon.fraser, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

Linux

Attachments:

Description	Flags
proposal patch	webkit.review.bot: commit-queue-
proposed patch	morrita: review+, webkit.review.bot: commit-queue-

younghwan cho

Reported 2012-02-27 02:31:55 PST

The shouldAntialiasLines function gets antialias OFF only for translations with no scale and Y-flipped. Three more conditions which do not need antialiasing are added in the shouldAntialiasLines function as belows. Integer Translation Integer Scaling X-flipped

Attachments
proposal patch (3.44 KB, patch) 2012-02-27 02:36 PST, younghwan cho	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
proposed patch (3.44 KB, patch) 2012-02-27 17:07 PST, younghwan cho	morrita: review+ webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

younghwan cho

Comment 1 2012-02-27 02:36:58 PST

Created attachment 128998 [details] proposal patch

WebKit Review Bot

Comment 2 2012-02-27 05:33:14 PST

Comment on attachment 128998 [details] proposal patch Attachment 128998 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11633901 New failing tests: css1/cascade/important.html css1/basic/class_as_selector.html css1/box_properties/border_color.html css1/box_properties/border_bottom.html accessibility/aria-disabled.html compositing/masks/multiple-masks.html css1/basic/grouping.html css1/box_properties/border_bottom_width.html css1/box_properties/border_bottom_width_inline.html css1/box_properties/border.html css1/basic/id_as_selector.html css1/basic/inheritance.html css1/box_properties/border_color_inline.html css1/classification/list_style_position.html css1/color_and_background/background_attachment.html css1/classification/list_style_image.html css1/classification/display.html css1/color_and_background/background.html compositing/reflections/nested-reflection-size-change.html css1/classification/list_style.html compositing/iframes/iframe-copy-on-scroll.html css1/box_properties/border_inline.html compositing/reflections/nested-reflection-opacity.html compositing/culling/filter-occlusion-blur.html css1/box_properties/border_bottom_inline.html css1/basic/containment.html css1/basic/contextual_selectors.html css1/basic/comments.html css1/cascade/cascade_order.html css1/classification/list_style_type.html

Simon Fraser (smfr)

Comment 3 2012-02-27 10:57:36 PST

Comment on attachment 128998 [details] proposal patch Can you supply more justification for this change? Is it a performance or quality improvement?

younghwan cho

Comment 4 2012-02-27 17:07:50 PST

Created attachment 129139 [details] proposed patch

younghwan cho

Comment 5 2012-02-27 21:56:44 PST

(In reply to comment #3) > (From update of attachment 128998 [details]) > Can you supply more justification for this change? Is it a performance or quality improvement? Drawing lines needs to decide that it should do or don't anti-aliasing. Current conditions of shouldAntialiasLines is not enough. so I added three conditions. and it can make better performance for graphics-engines. It's not about quality improvement.

WebKit Review Bot

Comment 6 2012-02-27 21:59:48 PST

Comment on attachment 129139 [details] proposed patch Attachment 129139 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11641415 New failing tests: css1/cascade/important.html css1/basic/class_as_selector.html css1/box_properties/border_color.html css1/box_properties/border_bottom.html accessibility/aria-disabled.html compositing/masks/multiple-masks.html css1/basic/grouping.html css1/box_properties/border_bottom_width.html css1/box_properties/border_bottom_width_inline.html css1/box_properties/border.html css1/basic/id_as_selector.html css1/basic/inheritance.html css1/box_properties/border_color_inline.html css1/classification/list_style_position.html css1/color_and_background/background_attachment.html css1/classification/list_style_image.html css1/classification/display.html css1/color_and_background/background.html compositing/reflections/nested-reflection-size-change.html css1/classification/list_style.html compositing/iframes/iframe-copy-on-scroll.html css1/box_properties/border_inline.html compositing/reflections/nested-reflection-opacity.html compositing/culling/filter-occlusion-blur.html css1/box_properties/border_bottom_inline.html css1/basic/containment.html css1/basic/contextual_selectors.html css1/basic/comments.html css1/cascade/cascade_order.html css1/classification/list_style_type.html

Igor Trindade Oliveira

Comment 7 2012-02-28 06:55:08 PST

when what it is the real gain? callgrind analysis? (In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 128998 [details] [details]) > > Can you supply more justification for this change? Is it a performance or quality improvement? > > Drawing lines needs to decide that it should do or don't anti-aliasing. > Current conditions of shouldAntialiasLines is not enough. > so I added three conditions. and it can make better performance for graphics-engines. It's not about quality improvement.

Hajime Morrita

Comment 8 2012-03-15 22:23:37 PDT

Comment on attachment 129139 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=129139&action=review r+ for now due to cr-linux redness. These redness means that this change actually affects the rendering result, doesn't it? > Source/WebCore/rendering/RenderBoxModelObject.cpp:2869 > + return !context->getCTM().isIdentityOrTranslationOrFlipped() || !context->getCTM().isIntegerTranslation() || !context->getCTM().isScaledByIntegerValue(); Some checks are apparently verbose.

Note You need to log in before you can comment on or make changes to this bug.