Summary: | [Qt] Dotted borders not drawn with rounded dots | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Martin Leutelt <martin.leutelt> | ||||||||||||||
Component: | Layout and Rendering | Assignee: | Martin Leutelt <martin.leutelt> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | bruno.abinader, dglazkov, igor.oliveira, noam, webkit.review.bot | ||||||||||||||
Priority: | P2 | Keywords: | Qt | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
URL: | http://www.w3.org/TR/css3-background/#border-style | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 25737, 98170 | ||||||||||||||||
Attachments: |
|
Description
Martin Leutelt
2012-07-26 07:27:27 PDT
Assigning to Martin, adding block to bug 25737. Created attachment 154657 [details]
Draw dotted borders with rounded dots
Attachment 154657 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/ChangeLog:6: Line contains tab character. [whitespace/tab] [5]
Total errors found: 1 in 3 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 154660 [details]
Draw dotted borders with rounded dots
Fixed style
Comment on attachment 154660 [details] Draw dotted borders with rounded dots Attachment 154660 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13351791 New failing tests: fast/forms/range/slider-delete-while-dragging-thumb.html css2.1/t170602-bdr-conflct-w-43-d.html css2.1/t170602-bdr-conflct-w-45-d.html css2.1/t170602-bdr-conflct-w-42-d.html fast/borders/outline-alpha-block.html css2.1/t0805-c5517-brdr-s-00-c.html fast/borders/borderRadiusAllStylesAllCorners.html css2.1/t170602-bdr-conflct-w-46-d.html css2.1/t170602-bdr-conflct-w-47-d.html css2.1/t170602-bdr-conflct-w-74-d.html css2.1/t170602-bdr-conflct-w-44-d.html fast/borders/borderRadiusDotted05.html css2.1/t170602-bdr-conflct-w-84-d.html css2.1/t170602-bdr-conflct-w-34-d.html css2.1/t170602-bdr-conflct-w-64-d.html css2.1/t170602-bdr-conflct-w-94-d.html fast/forms/range/slider-mouse-events.html fast/borders/border-mixed-alpha.html css2.1/t170602-bdr-conflct-w-54-d.html css1/box_properties/border_style.html css2.1/t170602-bdr-conflct-w-24-d.html fast/forms/control-clip.html css2.1/t170602-bdr-conflct-w-41-d.html css2.1/t170602-bdr-conflct-w-04-d.html css2.1/t170602-bdr-conflct-w-14-d.html css2.1/t170602-bdr-conflct-w-48-d.html css2.1/t170602-bdr-conflct-w-49-d.html Created attachment 154671 [details]
Archive of layout-test-results from gce-cr-linux-01
The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-01 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Created attachment 154910 [details]
Draw dotted borders with rounded dots
Patch now only changes GraphicsContextQt so other ports aren't influenced by changes made in GraphicsContext.
Comment on attachment 154910 [details]
Draw dotted borders with rounded dots
Requesting review and commit-queue flags.
Comment on attachment 154910 [details]
Draw dotted borders with rounded dots
We've just rebaselined plenty of pixel tests. Does this affect any of them? How is this tested?
I'm currently having a look into which tests would be affected or if there are any new tests needed. But I think at least some of the border related pixel tests would have to be rebaselined again if that patch was accepted. Created attachment 156274 [details]
Patch with updated expected results
Comment on attachment 156274 [details]
Patch with updated expected results
Requesting "review" and "commit-queue" flags.
Comment on attachment 156274 [details] Patch with updated expected results View in context: https://bugs.webkit.org/attachment.cgi?id=156274&action=review Looks good, But GraphicsContext::drawLine is growing to epic proportions. Let's try to split the dashing etc. to separate functions? > Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:376 > + if (isVerticalLine) { > + p1.setY(p1.y() - width / 2); > + p2.setY(p2.y() + width / 2); > + } else { > + p1.setX(p1.x() - width / 2); > + p2.setX(p2.x() + width / 2); > + } Can we wrap this with some function with a descriptive name? > Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:415 > + // Draw circles circles in the corners too. circles circles > Source/WebCore/platform/graphics/qt/GraphicsContextQt.cpp:419 > + if (style == DashedStroke) { > + capStyle = Qt::FlatCap; > + dashes << qreal(patWidth) / width << qreal(patWidth) / width; > + > + if (isVerticalLine) { > + p->fillRect(FloatRect(p1.x() - width / 2, p1.y() - width, width, width), QColor(color)); > + p->fillRect(FloatRect(p2.x() - width / 2, p2.y(), width, width), QColor(color)); > + } else { > + p->fillRect(FloatRect(p1.x() - width, p1.y() - width / 2, width, width), QColor(color)); > + p->fillRect(FloatRect(p2.x(), p2.y() - width / 2, width, width), QColor(color)); > + } > + } > + > + // As per css spec a dotted stroke should be made of circles. > + if (style == DottedStroke) { > + capStyle = Qt::RoundCap; > + // The actual length of one line element can not be set to zero and at 0.1 the dots > + // are still slightly elongated. Setting it to 0.01 will make it look like the > + // line endings are being stuck together, close enough to look like a circle. > + // For the distance of the line elements we subtract the small amount again. > + qreal lineElementLength = 0.01; > + dashes << lineElementLength << qreal(2 * patWidth) / width - lineElementLength; > + > + // Draw circles circles in the corners too. > + p->setPen(Qt::NoPen); > + p->setBrush(QColor(color)); > + p->drawEllipse(p1.x() - width / 2, p1.y() - width / 2, width, width); > + p->drawEllipse(p2.x() - width / 2, p2.y() - width / 2, width, width); Can we wrap this in a couple of functions with descriptive names? Created attachment 157643 [details]
Patch with updated expected results
New patch with requested changes and new expected results.
Comment on attachment 157643 [details]
Patch with updated expected results
Great, much more readable now!
Comment on attachment 157643 [details] Patch with updated expected results Clearing flags on attachment: 157643 Committed r125285: <http://trac.webkit.org/changeset/125285> All reviewed patches have been landed. Closing bug. |