RESOLVED INVALID 65484
Switch RoundedRect to use FloatRect
https://bugs.webkit.org/show_bug.cgi?id=65484
Summary Switch RoundedRect to use FloatRect
Levi Weintraub
Reported 2011-08-01 13:40:58 PDT
Taking a different tact for converting the graphics context to use floats instead of integers. Changing RoundedRect to use floats, and updating the relevant rendering and drawing code that operates on it.
Attachments
Work in Progress (Mac only) (40.71 KB, patch)
2011-08-01 13:42 PDT, Levi Weintraub
no flags
Patch (62.15 KB, patch)
2011-08-01 15:32 PDT, Levi Weintraub
no flags
Patch (62.41 KB, patch)
2011-08-01 17:10 PDT, Levi Weintraub
simon.fraser: review+
webkit-ews: commit-queue-
Build fixes for qt and linux (63.47 KB, patch)
2011-08-02 12:32 PDT, Levi Weintraub
webkit-ews: commit-queue-
More build fixes (64.78 KB, patch)
2011-08-02 16:28 PDT, Levi Weintraub
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02 (507.52 KB, application/zip)
2011-08-02 17:46 PDT, WebKit Review Bot
no flags
Fix for Ciaro, Qt, and Skia line border drawing (68.35 KB, patch)
2011-08-03 11:57 PDT, Levi Weintraub
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01 (507.13 KB, application/zip)
2011-08-03 12:39 PDT, WebKit Review Bot
no flags
Now working on all platforms. May cause a couple very small rendering differences. (72.43 KB, patch)
2011-08-16 11:09 PDT, Levi Weintraub
webkit-ews: commit-queue-
Had missed one local edit, d'oh (72.79 KB, patch)
2011-08-16 13:35 PDT, Levi Weintraub
webkit.review.bot: commit-queue-
Fixes to Path.cpp to properly clip to new roundedRects. (82.31 KB, patch)
2011-08-22 13:21 PDT, Levi Weintraub
no flags
New fixes updated to trunk (82.47 KB, patch)
2011-08-22 14:15 PDT, Levi Weintraub
gyuyoung.kim: commit-queue-
Build fixes (86.17 KB, patch)
2011-08-22 16:22 PDT, Levi Weintraub
webkit.review.bot: commit-queue-
Levi Weintraub
Comment 1 2011-08-01 13:42:56 PDT
Created attachment 102550 [details] Work in Progress (Mac only) I still need to update the platform GraphicsContext code for other platforms. This is currently passing all tests on Mac.
Early Warning System Bot
Comment 2 2011-08-01 13:58:16 PDT
Comment on attachment 102550 [details] Work in Progress (Mac only) Attachment 102550 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9285513
Gyuyoung Kim
Comment 3 2011-08-01 14:03:32 PDT
Comment on attachment 102550 [details] Work in Progress (Mac only) Attachment 102550 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9284531
WebKit Review Bot
Comment 4 2011-08-01 14:06:30 PDT
Comment on attachment 102550 [details] Work in Progress (Mac only) Attachment 102550 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9290501
Simon Fraser (smfr)
Comment 5 2011-08-01 14:06:48 PDT
Comment on attachment 102550 [details] Work in Progress (Mac only) View in context: https://bugs.webkit.org/attachment.cgi?id=102550&action=review > Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:-295 > - if (((int)width) % 2) { > - if (isVerticalLine) { > - // We're a vertical line. Adjust our x. > - p1.move(0.5f, 0.0f); > - p2.move(0.5f, 0.0f); > - } else { > - // We're a horizontal line. Adjust our y. > - p1.move(0.0f, 0.5f); > - p2.move(0.0f, 0.5f); > - } Are there layout tests that used to exercise this code? I'm a bit dubious about removing it in this patch.
Levi Weintraub
Comment 6 2011-08-01 14:15:53 PDT
(In reply to comment #5) > Are there layout tests that used to exercise this code? I'm a bit dubious about removing it in this patch. Yes there definitely are, namely the css2.1 border box tests. Read why this code is necessary in the comment I also removed. With this patch, WebKit doesn't truncate to integer when dividing by 2, which means we don't have to get that precision back in the platform layer.
Levi Weintraub
Comment 7 2011-08-01 15:32:18 PDT
Early Warning System Bot
Comment 8 2011-08-01 15:44:10 PDT
WebKit Review Bot
Comment 9 2011-08-01 16:01:32 PDT
Comment on attachment 102573 [details] Patch Attachment 102573 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9290531
Levi Weintraub
Comment 10 2011-08-01 16:27:27 PDT
(In reply to comment #9) > (From update of attachment 102573 [details]) > Attachment 102573 [details] did not pass chromium-ews (chromium-xvfb): > Output: http://queues.webkit.org/results/9290531 Looks like the Win bot is offline, but I'll re-upload build fixes once the rest of the bots process.
Levi Weintraub
Comment 11 2011-08-01 17:10:12 PDT
Early Warning System Bot
Comment 12 2011-08-01 17:27:47 PDT
WebKit Review Bot
Comment 13 2011-08-01 18:11:18 PDT
Comment on attachment 102594 [details] Patch Attachment 102594 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9287543
Levi Weintraub
Comment 14 2011-08-02 12:32:26 PDT
Created attachment 102682 [details] Build fixes for qt and linux
Early Warning System Bot
Comment 15 2011-08-02 12:52:07 PDT
Comment on attachment 102682 [details] Build fixes for qt and linux Attachment 102682 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9224069
WebKit Review Bot
Comment 16 2011-08-02 14:14:14 PDT
Comment on attachment 102682 [details] Build fixes for qt and linux Attachment 102682 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9290828
Levi Weintraub
Comment 17 2011-08-02 16:28:38 PDT
Created attachment 102712 [details] More build fixes
WebKit Review Bot
Comment 18 2011-08-02 17:46:07 PDT
Comment on attachment 102712 [details] More build fixes Attachment 102712 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9300090 New failing tests: css2.1/t170602-bdr-conflct-w-61-d.html css2.1/t170602-bdr-conflct-w-72-d.html css2.1/t170602-bdr-conflct-w-56-d.html css2.1/t170602-bdr-conflct-w-75-d.html css2.1/t170602-bdr-conflct-w-67-d.html css2.1/t0805-c5522-brdr-00-b.html css2.1/t170602-bdr-conflct-w-69-d.html css2.1/t170602-bdr-conflct-w-62-d.html css2.1/t170602-bdr-conflct-w-52-d.html css2.1/t170602-bdr-conflct-w-68-d.html css2.1/t170602-bdr-conflct-w-57-d.html css2.1/t170602-bdr-conflct-w-55-d.html css2.1/t170602-bdr-conflct-w-59-d.html css2.1/t170602-bdr-conflct-w-76-d.html css2.1/t170602-bdr-conflct-w-71-d.html css2.1/t0805-c5517-brdr-s-00-c.html css2.1/t170602-bdr-conflct-w-51-d.html css2.1/t170602-bdr-conflct-w-58-d.html css2.1/t170602-bdr-conflct-w-65-d.html css2.1/t170602-bdr-conflct-w-66-d.html
WebKit Review Bot
Comment 19 2011-08-02 17:46:12 PDT
Created attachment 102722 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Levi Weintraub
Comment 20 2011-08-03 11:57:49 PDT
Created attachment 102805 [details] Fix for Ciaro, Qt, and Skia line border drawing
WebKit Review Bot
Comment 21 2011-08-03 12:39:20 PDT
Comment on attachment 102805 [details] Fix for Ciaro, Qt, and Skia line border drawing Attachment 102805 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9305064 New failing tests: css2.1/t170602-bdr-conflct-w-61-d.html css2.1/t170602-bdr-conflct-w-72-d.html css2.1/t170602-bdr-conflct-w-56-d.html css2.1/t170602-bdr-conflct-w-75-d.html css2.1/t170602-bdr-conflct-w-67-d.html css2.1/t0805-c5522-brdr-00-b.html css2.1/t170602-bdr-conflct-w-69-d.html css2.1/t170602-bdr-conflct-w-62-d.html css2.1/t170602-bdr-conflct-w-52-d.html css2.1/t170602-bdr-conflct-w-68-d.html css2.1/t170602-bdr-conflct-w-57-d.html css2.1/t170602-bdr-conflct-w-55-d.html css2.1/t170602-bdr-conflct-w-59-d.html css2.1/t170602-bdr-conflct-w-76-d.html css2.1/t170602-bdr-conflct-w-71-d.html css2.1/t0805-c5517-brdr-s-00-c.html css2.1/t170602-bdr-conflct-w-51-d.html css2.1/t170602-bdr-conflct-w-58-d.html css2.1/t170602-bdr-conflct-w-65-d.html css2.1/t170602-bdr-conflct-w-66-d.html
WebKit Review Bot
Comment 22 2011-08-03 12:39:35 PDT
Created attachment 102813 [details] Archive of layout-test-results from ec2-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-01 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Levi Weintraub
Comment 23 2011-08-16 11:09:04 PDT
Created attachment 104069 [details] Now working on all platforms. May cause a couple very small rendering differences. Will re-request a review pending results from the cr-linux bot.
Early Warning System Bot
Comment 24 2011-08-16 11:29:45 PDT
Comment on attachment 104069 [details] Now working on all platforms. May cause a couple very small rendering differences. Attachment 104069 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9408231
Gyuyoung Kim
Comment 25 2011-08-16 11:31:26 PDT
Comment on attachment 104069 [details] Now working on all platforms. May cause a couple very small rendering differences. Attachment 104069 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9408233
WebKit Review Bot
Comment 26 2011-08-16 11:39:10 PDT
Comment on attachment 104069 [details] Now working on all platforms. May cause a couple very small rendering differences. Attachment 104069 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9407307
Collabora GTK+ EWS bot
Comment 27 2011-08-16 12:15:45 PDT
Comment on attachment 104069 [details] Now working on all platforms. May cause a couple very small rendering differences. Attachment 104069 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9414083
Levi Weintraub
Comment 28 2011-08-16 13:35:18 PDT
Created attachment 104082 [details] Had missed one local edit, d'oh
WebKit Review Bot
Comment 29 2011-08-17 22:07:26 PDT
Comment on attachment 104082 [details] Had missed one local edit, d'oh Attachment 104082 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9425161 New failing tests: fast/css/background-clip-values.html fast/forms/input-number-events.html media/audio-controls-rendering.html fast/backgrounds/gradient-background-leakage.html fast/transforms/shadows.html fast/borders/borderRadiusDashed02.html fast/layers/video-layer.html fast/backgrounds/repeat/negative-offset-repeat-transformed.html fast/borders/border-radius-constraints.html fast/gradients/background-clipped.html fast/borders/border-radius-huge-assert.html fast/forms/input-spinbutton-capturing.html fast/forms/input-number-large-padding.html css2.1/t0805-c5517-brdr-s-00-c.html fast/borders/borderRadiusDotted01.html fast/forms/implicit-submission.html
Levi Weintraub
Comment 30 2011-08-22 13:21:12 PDT
Created attachment 104725 [details] Fixes to Path.cpp to properly clip to new roundedRects.
Levi Weintraub
Comment 31 2011-08-22 14:15:11 PDT
Created attachment 104736 [details] New fixes updated to trunk
WebKit Review Bot
Comment 32 2011-08-22 14:21:08 PDT
Attachment 104736 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/inspector/DOMNodeHighlighte..." exit_code: 1 Source/WebCore/platform/graphics/RoundedRect.cpp:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:44: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:52: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:53: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:54: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:60: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:62: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:63: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 12 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gyuyoung Kim
Comment 33 2011-08-22 15:32:11 PDT
Comment on attachment 104736 [details] New fixes updated to trunk Attachment 104736 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/9468877
Levi Weintraub
Comment 34 2011-08-22 16:22:50 PDT
Created attachment 104761 [details] Build fixes
WebKit Review Bot
Comment 35 2011-08-22 16:27:12 PDT
Attachment 104761 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/inspector/DOMNodeHighlighte..." exit_code: 1 Source/WebCore/platform/graphics/RoundedRect.cpp:43: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:44: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:49: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:52: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:53: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:54: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:56: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:57: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:59: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:60: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:62: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/RoundedRect.cpp:63: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 12 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 36 2011-08-22 21:43:56 PDT
Comment on attachment 104761 [details] Build fixes Attachment 104761 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/9476180 New failing tests: media/video-no-audio.html media/controls-strict.html media/controls-styling.html media/video-display-toggle.html media/audio-repaint.html media/audio-controls-rendering.html fast/transforms/shadows.html media/video-controls-rendering.html fast/borders/border-radius-constraints.html media/controls-without-preload.html media/media-controls-clone.html fast/layers/video-layer.html media/video-empty-source.html fast/forms/implicit-submission.html media/controls-after-reload.html
Note You need to log in before you can comment on or make changes to this bug.