65484 – Switch RoundedRect to use FloatRect

Bug 65484 - Switch RoundedRect to use FloatRect

Summary: Switch RoundedRect to use FloatRect

Status:	RESOLVED INVALID

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	Layout and Rendering (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Levi Weintraub

URL:
Keywords:

Depends on:
Blocks:	64405
	Show dependency tree / graph

Reported:	2011-08-01 13:40 PDT by Levi Weintraub
Modified:	2011-11-01 17:05 PDT (History)
CC List:	9 users (show)

See Also:

Attachments
Work in Progress (Mac only) (40.71 KB, patch) 2011-08-01 13:42 PDT, Levi Weintraub	no flags	Details \| Formatted Diff \| Diff
Patch (62.15 KB, patch) 2011-08-01 15:32 PDT, Levi Weintraub	no flags	Details \| Formatted Diff \| Diff
Patch (62.41 KB, patch) 2011-08-01 17:10 PDT, Levi Weintraub	simon.fraser: review+ webkit-ews: commit-queue-	Details \| Formatted Diff \| Diff
Build fixes for qt and linux (63.47 KB, patch) 2011-08-02 12:32 PDT, Levi Weintraub	webkit-ews: commit-queue-	Details \| Formatted Diff \| Diff
More build fixes (64.78 KB, patch) 2011-08-02 16:28 PDT, Levi Weintraub	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
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	Details
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-	Details \| Formatted Diff \| Diff
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	Details
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-	Details \| Formatted Diff \| Diff
Had missed one local edit, d'oh (72.79 KB, patch) 2011-08-16 13:35 PDT, Levi Weintraub	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
Fixes to Path.cpp to properly clip to new roundedRects. (82.31 KB, patch) 2011-08-22 13:21 PDT, Levi Weintraub	no flags	Details \| Formatted Diff \| Diff
New fixes updated to trunk (82.47 KB, patch) 2011-08-22 14:15 PDT, Levi Weintraub	gyuyoung.kim: commit-queue-	Details \| Formatted Diff \| Diff
Build fixes (86.17 KB, patch) 2011-08-22 16:22 PDT, Levi Weintraub	webkit.review.bot: commit-queue-	Details \| Formatted Diff \| Diff
Show Obsolete (9) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Levi Weintraub 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.

Comment 1 Levi Weintraub 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.

Comment 2 Early Warning System Bot 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

Comment 3 Gyuyoung Kim 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

Comment 4 WebKit Review Bot 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

Comment 5 Simon Fraser (smfr) 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.

Comment 6 Levi Weintraub 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.

Comment 7 Levi Weintraub 2011-08-01 15:32:18 PDT

Created attachment 102573 [details]
Patch

Comment 8 Early Warning System Bot 2011-08-01 15:44:10 PDT

Comment on attachment 102573 [details]
Patch

Attachment 102573 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9286529

Comment 9 WebKit Review Bot 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

Comment 10 Levi Weintraub 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.

Comment 11 Levi Weintraub 2011-08-01 17:10:12 PDT

Created attachment 102594 [details]
Patch

Comment 12 Early Warning System Bot 2011-08-01 17:27:47 PDT

Comment on attachment 102594 [details]
Patch

Attachment 102594 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9288591

Comment 13 WebKit Review Bot 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

Comment 14 Levi Weintraub 2011-08-02 12:32:26 PDT

Created attachment 102682 [details]
Build fixes for qt and linux

Comment 15 Early Warning System Bot 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

Comment 16 WebKit Review Bot 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

Comment 17 Levi Weintraub 2011-08-02 16:28:38 PDT

Created attachment 102712 [details]
More build fixes

Comment 18 WebKit Review Bot 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

Comment 19 WebKit Review Bot 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

Comment 20 Levi Weintraub 2011-08-03 11:57:49 PDT

Created attachment 102805 [details]
Fix for Ciaro, Qt, and Skia line border drawing

Comment 21 WebKit Review Bot 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

Comment 22 WebKit Review Bot 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

Comment 23 Levi Weintraub 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.

Comment 24 Early Warning System Bot 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

Comment 25 Gyuyoung Kim 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

Comment 26 WebKit Review Bot 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

Comment 27 Collabora GTK+ EWS bot 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

Comment 28 Levi Weintraub 2011-08-16 13:35:18 PDT

Created attachment 104082 [details]
Had missed one local edit, d'oh

Comment 29 WebKit Review Bot 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

Comment 30 Levi Weintraub 2011-08-22 13:21:12 PDT

Created attachment 104725 [details]
Fixes to Path.cpp to properly clip to new roundedRects.

Comment 31 Levi Weintraub 2011-08-22 14:15:11 PDT

Created attachment 104736 [details]
New fixes updated to trunk

Comment 32 WebKit Review Bot 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.

Comment 33 Gyuyoung Kim 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

Comment 34 Levi Weintraub 2011-08-22 16:22:50 PDT

Created attachment 104761 [details]
Build fixes

Comment 35 WebKit Review Bot 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.

Comment 36 WebKit Review Bot 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