Bug 136264 - Subpixel layout: Cleanup snapSizeToPixel/snapSizeToDevicePixel.
Summary: Subpixel layout: Cleanup snapSizeToPixel/snapSizeToDevicePixel.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
Depends on:
Blocks: 136275
  Show dependency tree / graph
 
Reported: 2014-08-26 12:35 PDT by zalan
Modified: 2014-08-27 22:27 PDT (History)
5 users (show)

See Also:

Attachments
Patch (15.07 KB, patch)
2014-08-26 12:41 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (92.32 KB, patch)
2014-08-26 20:50 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (92.99 KB, patch)
2014-08-27 13:45 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (92.77 KB, patch)
2014-08-27 21:40 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (92.77 KB, patch)
2014-08-27 21:46 PDT, zalan 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) 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 zalan 2014-08-26 12:35:21 PDT 
Pixel in the function name (without specifying whether its a device or CSS pixel) is misleading.
Comment 1 zalan 2014-08-26 12:41:28 PDT 
Created attachment 237166 [details]
Patch
Comment 2 Simon Fraser (smfr) 2014-08-26 13:19:28 PDT 
Comment on attachment 237166 [details]
Patch

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

r- because snapSize with a local point is meaningless.

> Source/WebCore/platform/LayoutUnit.h:919
> +inline int snapSizeToInt(LayoutUnit size, LayoutUnit location)

Three things make me sad about this. "snap" outside of pixel snapping, "size" which is not a LayoutSize and "location" that is not a layout point.

Can we just remove this and down the work inside LayoutRect?
Comment 3 zalan 2014-08-26 20:50:24 PDT 
Created attachment 237198 [details]
Patch
Comment 4 WebKit Commit Bot 2014-08-26 20:51:08 PDT 
Attachment 237198 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/LayoutPoint.h:184:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/LayoutPoint.h:210:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 zalan 2014-08-27 13:45:03 PDT 
Created attachment 237246 [details]
Patch
Comment 6 WebKit Commit Bot 2014-08-27 13:46:26 PDT 
Attachment 237246 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/LayoutPoint.h:184:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/platform/graphics/LayoutPoint.h:209:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 zalan 2014-08-27 21:40:06 PDT 
Created attachment 237287 [details]
Patch
Comment 8 WebKit Commit Bot 2014-08-27 21:43:41 PDT 
Comment on attachment 237287 [details]
Patch

Rejecting attachment 237287 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 237287, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Tools/ChangeLog contains OOPS!.

Full output: http://webkit-queues.appspot.com/results/4801182182670336
Comment 9 zalan 2014-08-27 21:46:31 PDT 
Created attachment 237288 [details]
Patch
Comment 10 WebKit Commit Bot 2014-08-27 22:27:52 PDT 
Comment on attachment 237288 [details]
Patch

Clearing flags on attachment: 237288

Committed r173049: <http://trac.webkit.org/changeset/173049>
Comment 11 WebKit Commit Bot 2014-08-27 22:27:55 PDT 
All reviewed patches have been landed.  Closing bug.