Bug 98548 - [CSS Exclusions] Additional simple polygon tests
Summary: [CSS Exclusions] Additional simple polygon tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hans Muller
URL:
Keywords:
Depends on: 96811
Blocks: 99342 99343
  Show dependency treegraph
 
Reported: 2012-10-05 12:31 PDT by Hans Muller
Modified: 2012-10-17 09:27 PDT (History)
4 users (show)

See Also:


Attachments
Patch (16.86 KB, patch)
2012-10-10 10:25 PDT, Hans Muller
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (26.98 KB, patch)
2012-10-10 14:42 PDT, Hans Muller
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (27.04 KB, patch)
2012-10-11 08:38 PDT, Hans Muller
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (27.18 KB, patch)
2012-10-11 14:42 PDT, Hans Muller
krit: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (27.18 KB, patch)
2012-10-16 15:41 PDT, Hans Muller
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (27.18 KB, patch)
2012-10-16 15:47 PDT, Hans Muller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hans Muller 2012-10-05 12:31:34 PDT
A set of tests that check the support for polygonal shape-inside exclusions is needed.  Currently shape-inside layout assumes that the top and bottom edges of the polygon are horizontal and that lines that overlap the polygon only cross the polygon twice.  A set of tests that checks this subset of the general case is needed.
Comment 1 Hans Muller 2012-10-10 10:25:29 PDT
Created attachment 168026 [details]
Patch

Added a pair of CSS Exclusions shape-inside ref-tests for regular polygons with 8 and 16 sides. Also made some small revisions to the supporting code, simple-polyon.js, and the existing simple polygon tests, to localize the conversion from float to layout coordinates and to cleanup the extraneous vertices argument int the createPolygonShapeInsideTestCase functions.
Comment 2 WebKit Review Bot 2012-10-10 11:21:51 PDT
Comment on attachment 168026 [details]
Patch

Attachment 168026 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14260099

New failing tests:
fast/exclusions/shape-inside/shape-inside-regular-polygon16.html
fast/exclusions/shape-inside/shape-inside-regular-polygon8.html
Comment 3 Zoltan Horvath 2012-10-10 14:41:48 PDT
(In reply to comment #2)
> (From update of attachment 168026 [details])
> Attachment 168026 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/14260099
> 
> New failing tests:
> fast/exclusions/shape-inside/shape-inside-regular-polygon16.html
> fast/exclusions/shape-inside/shape-inside-regular-polygon8.html

It seems we need to generate pixel test results for these. Otherwise the patch looks okay to me.
Comment 4 Hans Muller 2012-10-10 14:42:30 PDT
Created attachment 168074 [details]
Patch

Detect subpixel layout support hand handle rounding the left edge of line segment intervals by actually rounding in that case.
Comment 5 WebKit Review Bot 2012-10-10 20:03:15 PDT
Comment on attachment 168074 [details]
Patch

Attachment 168074 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14258397

New failing tests:
fast/exclusions/shape-inside/shape-inside-regular-polygon8.html
Comment 6 Hans Muller 2012-10-11 08:38:02 PDT
Created attachment 168232 [details]
Patch

Revised simple-polygon.js subpixelRound() function to more closely match FractionLayout(floatValue).round().
Comment 7 WebKit Review Bot 2012-10-11 09:26:57 PDT
Comment on attachment 168232 [details]
Patch

Attachment 168232 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14264015

New failing tests:
fast/exclusions/shape-inside/shape-inside-regular-polygon8.html
Comment 8 Hans Muller 2012-10-11 14:42:09 PDT
Created attachment 168280 [details]
Patch

After testing on a Chromium build, corrected two problems: regular polygons were not always exactly symmetric vis the Y axis, shape-inside-regular-polygon8-expected.html was not initializing hasSubpixelSupport.
Comment 9 WebKit Review Bot 2012-10-12 19:10:13 PDT
Comment on attachment 168280 [details]
Patch

Rejecting attachment 168280 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
ueue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: http://queues.webkit.org/results/14287113
Comment 10 Dirk Schulze 2012-10-16 13:46:38 PDT
Comment on attachment 168280 [details]
Patch

Lets try it again.
Comment 11 WebKit Review Bot 2012-10-16 13:53:51 PDT
Comment on attachment 168280 [details]
Patch

Rejecting attachment 168280 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

Last 500 characters of output:
ueue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 70, in run_and_handle_errors
    self._run(tool, options, state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/commands/stepsequence.py", line 64, in _run
    step(tool, options).run(state)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in run
    if changelog_entry.has_valid_reviewer():
AttributeError: 'NoneType' object has no attribute 'has_valid_reviewer'

Full output: http://queues.webkit.org/results/14388301
Comment 12 Hans Muller 2012-10-16 15:41:36 PDT
Created attachment 169043 [details]
Patch

Advanced ChangeLog entry date by 1000 years.
Comment 13 WebKit Review Bot 2012-10-16 15:44:35 PDT
Comment on attachment 169043 [details]
Patch

Rejecting attachment 169043 [details] from commit-queue.

giles_joplin@yahoo.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 14 Hans Muller 2012-10-16 15:46:14 PDT
(In reply to comment #12)
> Created an attachment (id=169043) [details]
> Patch
> 
> Advanced ChangeLog entry date by 1000 years.

Sorry.  That's 2000 years.  Hard to keep the millennia straight.
Comment 15 Hans Muller 2012-10-16 15:47:36 PDT
Created attachment 169044 [details]
Patch

Resubmitting after accidentally setting the commit flag myself.
Comment 16 WebKit Review Bot 2012-10-17 09:27:11 PDT
Comment on attachment 169044 [details]
Patch

Clearing flags on attachment: 169044

Committed r131610: <http://trac.webkit.org/changeset/131610>
Comment 17 WebKit Review Bot 2012-10-17 09:27:15 PDT
All reviewed patches have been landed.  Closing bug.