Bug 149366 - Subpixel layout: Convert RenderTable* and AutoTableLayout to use LayoutUnit.
Summary: Subpixel layout: Convert RenderTable* and AutoTableLayout to use LayoutUnit.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords:
: 149332 (view as bug list)
Depends on: 150616
Blocks: 149935
  Show dependency tree / graph
 
Reported: 2015-09-18 20:30 PDT by zalan
Modified: 2016-09-28 17:31 PDT (History)
9 users (show)

See Also:

Attachments
Patch (87.69 KB, patch)
2015-09-18 20:37 PDT, zalan 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews102 for mac-mavericks (310.86 KB, application/zip)
2015-09-18 21:06 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (287.83 KB, application/zip)
2015-09-18 21:09 PDT, Build Bot 		no flags Details
Patch (3.76 MB, patch)
2015-09-26 18:09 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (3.76 MB, patch)
2015-09-26 18:46 PDT, zalan 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (746.07 KB, application/zip)
2015-09-26 19:29 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews106 for mac-mavericks-wk2 (779.78 KB, application/zip)
2015-09-26 19:34 PDT, Build Bot 		no flags Details
Patch (8.81 MB, patch)
2015-10-19 15:47 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch for EWS (18.38 MB, patch)
2015-10-20 10:49 PDT, zalan 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-yosemite (1020.67 KB, application/zip)
2015-10-20 13:21 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews101 for mac-mavericks (772.62 KB, application/zip)
2015-10-20 13:25 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (786.40 KB, application/zip)
2015-10-20 13:30 PDT, Build Bot 		no flags Details
Patch for EWS (18.53 MB, text/plain)
2015-10-20 15:57 PDT, zalan 		no flags Details
Patch for EWS (18.54 MB, patch)
2015-10-20 19:32 PDT, zalan 		no flags Details | Formatted Diff | Diff
Patch (code change only) for reviewing (114.61 KB, patch)
2015-10-21 09:15 PDT, zalan 		hyatt: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-yosemite (431.20 KB, application/zip)
2015-10-21 09:55 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews102 for mac-mavericks (501.48 KB, application/zip)
2015-10-21 09:56 PDT, Build Bot 		no flags Details
Archive of layout-test-results from ews104 for mac-mavericks-wk2 (435.95 KB, application/zip)
2015-10-21 09:58 PDT, Build Bot 		no flags Details
Show Obsolete (16) 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 2015-09-18 20:30:16 PDT 
ssia.
Comment 1 zalan 2015-09-18 20:37:35 PDT 
Created attachment 261565 [details]
Patch
Comment 2 zalan 2015-09-18 20:37:55 PDT 
Comment on attachment 261565 [details]
Patch

EWS testing.
Comment 3 Build Bot 2015-09-18 21:06:47 PDT 
Comment on attachment 261565 [details]
Patch

Attachment 261565 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/185310

Number of test failures exceeded the failure limit.
Comment 4 Build Bot 2015-09-18 21:06:51 PDT 
Created attachment 261568 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 5 Build Bot 2015-09-18 21:09:30 PDT 
Comment on attachment 261565 [details]
Patch

Attachment 261565 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/185311

Number of test failures exceeded the failure limit.
Comment 6 Build Bot 2015-09-18 21:09:34 PDT 
Created attachment 261569 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 7 Darin Adler 2015-09-19 15:49:59 PDT 
Comment on attachment 261565 [details]
Patch

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

> Source/WebCore/rendering/AutoTableLayout.cpp:246
> +        maxWidth = std::max(maxWidth, std::min<LayoutUnit>(maxNonPercent, tableMaxWidth));
> +        maxWidth = std::max(maxWidth, std::min<LayoutUnit>(maxPercent, tableMaxWidth));

I don’t understand why explicit <LayoutUnit> here is needed or why it’s safe.

> Source/WebCore/rendering/AutoTableLayout.cpp:256
> +        minWidth = maxWidth = std::max<LayoutUnit>(minWidth, tableLogicalWidth.value());

I don’t understand why explicit <LayoutUnit> here is needed or why it’s safe.

> Source/WebCore/rendering/AutoTableLayout.cpp:349
> +                maxLogicalWidth = std::max<LayoutUnit>(maxLogicalWidth, std::max(spanMaxLogicalWidth, cellMaxLogicalWidth) * 100  / cellLogicalWidth.percent());

I don’t understand why explicit <LayoutUnit> here is needed or why it’s safe.

Noticed two spaces between "100" and "/".

> Source/WebCore/rendering/AutoTableLayout.cpp:377
> +                    LayoutUnit cellLogicalWidth = std::max<LayoutUnit>(m_layoutStruct[pos].effectiveMinLogicalWidth, cellMinLogicalWidth * m_layoutStruct[pos].logicalWidth.value() / fixedWidth);

I don’t understand why explicit <LayoutUnit> here is needed or why it’s safe.
Comment 8 zalan 2015-09-26 18:09:52 PDT 
Created attachment 261981 [details]
Patch
Comment 9 zalan 2015-09-26 18:10:20 PDT 
EWS testing.
Comment 10 zalan 2015-09-26 18:46:18 PDT 
Created attachment 261982 [details]
Patch
Comment 11 Build Bot 2015-09-26 19:29:53 PDT 
Comment on attachment 261982 [details]
Patch

Attachment 261982 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/215728

Number of test failures exceeded the failure limit.
Comment 12 Build Bot 2015-09-26 19:29:57 PDT 
Created attachment 261983 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 13 Build Bot 2015-09-26 19:34:50 PDT 
Comment on attachment 261982 [details]
Patch

Attachment 261982 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/215732

Number of test failures exceeded the failure limit.
Comment 14 Build Bot 2015-09-26 19:34:53 PDT 
Created attachment 261984 [details]
Archive of layout-test-results from ews106 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 15 zalan 2015-10-19 15:47:52 PDT 
Created attachment 263521 [details]
Patch
Comment 16 zalan 2015-10-20 10:49:10 PDT 
Created attachment 263591 [details]
Patch for EWS
Comment 17 WebKit Commit Bot 2015-10-20 11:50:42 PDT 
Attachment 263591 [details] did not pass style-queue:

Traceback (most recent call last):
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module>
    main()
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main
    WebKitPatch(os.path.abspath(__file__)).main()
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 54, in execute
    self._sequence.run_and_handle_errors(tool, options, state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run
    step(tool, options).run(state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 65, in run
    self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 131, in run_and_throw_if_fail
    exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 103, in _run_command_with_teed_output
    **kwargs)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 506, in popen
    return subprocess.Popen(string_args, **kwargs)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1308, in _execute_child
    raise child_exception
OSError: [Errno 7] Argument list too long


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 18 Build Bot 2015-10-20 13:21:10 PDT 
Comment on attachment 263591 [details]
Patch for EWS

Attachment 263591 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/312727

New failing tests:
fast/repaint/table-hover-on-link.html
tables/mozilla_expected_failures/bugs/bug89315.html
tables/mozilla_expected_failures/bugs/bug8499.html
Comment 19 Build Bot 2015-10-20 13:21:18 PDT 
Created attachment 263608 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 20 Build Bot 2015-10-20 13:25:47 PDT 
Created attachment 263609 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 21 Build Bot 2015-10-20 13:30:13 PDT 
Created attachment 263610 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 22 zalan 2015-10-20 15:57:12 PDT 
Created attachment 263627 [details]
Patch for EWS
Comment 23 zalan 2015-10-20 19:32:22 PDT 
Created attachment 263651 [details]
Patch for EWS
Comment 24 WebKit Commit Bot 2015-10-20 19:39:26 PDT 
Attachment 263651 [details] did not pass style-queue:

Traceback (most recent call last):
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 84, in <module>
    main()
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkit-patch", line 79, in main
    WebKitPatch(os.path.abspath(__file__)).main()
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 305, in main
    result = command.check_arguments_and_execute(options, args, self)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/multicommandtool.py", line 123, in check_arguments_and_execute
    return self.execute(options, args, tool) or 0
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/abstractsequencedcommand.py", line 54, in execute
    self._sequence.run_and_handle_errors(tool, options, state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 73, in run_and_handle_errors
    self._run(tool, options, state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 67, in _run
    step(tool, options).run(state)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/tool/steps/checkstyle.py", line 65, in run
    self._tool.executive.run_and_throw_if_fail(self._tool.deprecated_port().check_webkit_style_command() + args, cwd=self._tool.scm().checkout_root)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 131, in run_and_throw_if_fail
    exit_code = self._run_command_with_teed_output(args, child_stdout, **kwargs)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 103, in _run_command_with_teed_output
    **kwargs)
  File "/Volumes/Data/StyleQueue/WebKit/Tools/Scripts/webkitpy/common/system/executive.py", line 506, in popen
    return subprocess.Popen(string_args, **kwargs)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 711, in __init__
    errread, errwrite)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1308, in _execute_child
    raise child_exception
OSError: [Errno 7] Argument list too long


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 25 zalan 2015-10-21 09:15:51 PDT 
Created attachment 263683 [details]
Patch (code change only) for reviewing
Comment 26 Build Bot 2015-10-21 09:55:27 PDT 
Comment on attachment 263683 [details]
Patch (code change only) for reviewing

Attachment 263683 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/316512

Number of test failures exceeded the failure limit.
Comment 27 Build Bot 2015-10-21 09:55:31 PDT 
Created attachment 263686 [details]
Archive of layout-test-results from ews117 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 28 Build Bot 2015-10-21 09:56:07 PDT 
Comment on attachment 263683 [details]
Patch (code change only) for reviewing

Attachment 263683 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/316519

Number of test failures exceeded the failure limit.
Comment 29 Build Bot 2015-10-21 09:56:10 PDT 
Created attachment 263687 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 30 Build Bot 2015-10-21 09:58:04 PDT 
Comment on attachment 263683 [details]
Patch (code change only) for reviewing

Attachment 263683 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/316518

Number of test failures exceeded the failure limit.
Comment 31 Build Bot 2015-10-21 09:58:08 PDT 
Created attachment 263688 [details]
Archive of layout-test-results from ews104 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 32 Dave Hyatt 2015-10-21 11:02:19 PDT 
Comment on attachment 263683 [details]
Patch (code change only) for reviewing

r=me
Comment 33 zalan 2015-10-22 10:49:02 PDT 
I'll land this patch soon after I have the Win results.
Comment 34 zalan 2015-10-28 10:48:47 PDT 
*** Bug 149332 has been marked as a duplicate of this bug. ***
Comment 36 John Bridges 2016-09-28 17:31:10 PDT 
Still appears to be rounding the start X/Y of each cell to integer.
View this in current iOS Safari, pinch/zoom and you will see hairlines appear between rows. Same with other WebKit browsers/Email clients.
If if were only off by 1/64th of a pixel, the lines would be far more faint.

<!DOCTYPE html>
<html><body><table width="120" cellspacing="0" cellpadding="0" height="99"><tbody>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
<tr><td bgcolor="#005486"></td></tr>
</tbody></table></body></html>