Bug 56928 – [Qt] Bottom border of an inline is cut if it is 1 pixel wide and left or right border is thicker.

• Component: CSS
• Status: RESOLVED INVALID

[Yael]

Created attachment 86624 [details]
Test case

GraphicsContext::drawConvexPolygon() is supposed to draw a trapezoid and it does it properly for the top border. If the bottom border is 1 pixel wide, only the shorter part of the trapezoid is drawn, so part of the border is missing.

I actually think this is a bug in QPainter::drawConvexPolygon(), but wanted to check here first.

[Yael]

(In reply to comment #0)
> Created an attachment (id=86624) [details]
> Test case
> I actually think this is a bug in QPainter::drawConvexPolygon(), but wanted to check here first.

Removing the antialiasing hint also fixes this problem.

[Yael]

Created attachment 86640 [details]
Patch.

[Benjamin Poulain]

Comment on attachment 86640 [details]
Patch.

We should not make workaround in WebKit for bugs in the lower layers. We should fix the lower layer code.
Of course sometimes it is not possible, but here you did not give any reason why we could not fix the graphics engine.

[Benjamin Poulain]

Which graphics engine has the bug?

[Yael]

Created attachment 86667 [details]
Patch.

Before turning off antialiasing, I thought this is a bug in QPainter. But it probably isn't.
because configuring QPainter by turning off antialiasing fixes this issue.

[Yael]

Comment on attachment 86667 [details]
Patch.

Removing the review flag, looks like I messed up :)

[Yael]

Created attachment 86684 [details]
Patch.

Turns out that we need to enable. not disable antialiasing in this case.

[Yael]

Created attachment 86687 [details]
Patch.

Same patch, just added mac results.

[Andreas Kling]

Comment on attachment 86687 [details]
Patch.

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

> Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:456
> +    // Force antialiasing hint when drawing a border of 1 pixel width
> +    bool forceAntialiasHint = (npoints == 4 && polygon.boundingRect().height() == 1);

Since this is a workaround for a bug in Qt's X11 paint engine, I think we should make that clear by either:

A) Making the workaround QT_WS_X11 and QPaintEngine::X11 specific (just like drawLineForText does.)
B) Including a comment about why we're doing this.
C) Both.

I personally prefer C, but all are fine with me. :)

[Benjamin Poulain]

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

I really don't like that. That means polygons will have antialiasing or not depending on arbitrary conditions.

First, could you explain why the problem exists, and why antialiasing fixes it?

> Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:455
> +    // Force antialiasing hint when drawing a border of 1 pixel width

This comment does not add any value, because it does not say why it is needed.
The next line has the same information as the comment.
<Kenneth>And the sentence must end with a dot to be a sentence</Kenneth> :)

> Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:462
> +    if (forceAntialiasHint)
> +        p->setRenderHint(QPainter::Antialiasing, true);
> +    else
> +        p->setRenderHint(QPainter::Antialiasing, shouldAntialias);

This won't fly. I am not against enabling antialiasing by default if there is a good reason for it, but enabling it for a special case is not a good idea.
Don't the default render hint include antialiasing?

[Yael]

Created attachment 86714 [details]
Patch.

Added a flag for Q_WS_X11.
I wanted to also test for p->paintEngine()->type() == QPaintEngine::X11, but adding that test casued the layout test to fail.

[Benjamin Poulain]

Comment on attachment 86714 [details]
Patch.

First we need the reason why the path is not rendered correct without antialiasing.
A X11 paint engine hack could be acceptable, this is not was you are proposing with this last patch.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.