Bug 36444

Summary: [Qt] LayoutTests/fast/canvas/fillrect_gradient.html failed
Product: WebKit Reporter: qi <qi.2.zhang>
Component: New BugsAssignee: qi <qi.2.zhang>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, cshu, hausmann, laszlo.gombos
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
fix platformGradient
hausmann: review-
use sortStopsIfNecessary none

Description qi 2010-03-22 08:30:05 PDT
QtLauncher fails on the following test case while Safari works.
  LayoutTests/fast/canvas/fillrect_gradient.html

Actually, it failed on the last 2 test item in this case.
Comment 1 qi 2010-03-22 11:09:15 PDT
Created attachment 51312 [details]
fix platformGradient

Based on HTML5.0 (4.8.10.1.4 Colors and styles)

The addColorStop(offset, color) method on the CanvasGradient interface adds a new stop to a gradient.
If the offset is less than 0, greater than 1, infinite, or NaN, then an INDEX_SIZE_ERR exception must be raised. If
the color cannot be parsed as a CSS color, then a SYNTAX_ERR exception must be raised. Otherwise, the
gradient must have a new stop placed, at offset offset relative to the whole gradient, and with the color obtained
by parsing color as a CSS <color> value. If multiple stops are added at the same offset on a gradient, they must
be placed in the order added, with the first one closest to the start of the gradient, and each subsequent one
infinitesimally further along towards the end point (in effect causing all but the first and last stop added at each
point to be ignored).

1. Currently, the algorithm of the platformGradient depends on a sorted colorStop array, otherwise when the duplicated colorStop are not continually, it will failed, because it can't find the duplicated colorStop anymore.
2. When we have duplicated colorStop, suppose only the first and last works, the other shall be ignored based on the spec.
for example, in this test case, we have case like:
colorStop      current           should be
0,  '#00f'     --> 0             -->0
0.5,'#fff'     -->0.5            -->0.5
0.5,'#aaa'     -->0.5+0.00...1   -->0.5+0.00...1                 -->finally, it is ignored
0.5,'#abc'     -->0.5+0.00...2   -->0.5+0.00...1(overwrite pre)  --> finally, it ignored
0.5,'#f00'     -->0.5+0.00...3   -->0.5+0.00...1(overwrite pre)
1,  '#ff0'     -->1              -->1
another case is:
colorStop                current                                                  should be
0.5,   '#fff'            0.5,  '#fff'    -->finally, it is ignored                0, '#00f'
0.5,   '#aaa'            0.5+0.00...1, '#aaa'  -->finally, it is ignored          0.5,  '#fff'
1,     '#ff0'            1, '#ff0'                                                0.5+0.00...1,'#aaa'         -->finally, it is ignored
0.5,   '#abc'            0.5, '#abc'  (overwrite the 1th)                         0.5+0.00...1,'#abc'(overwrite pre) -->finally, it is ignored
0.5,   '#f00'            0.5+0.00...1, '#f00' (overwrite the 2nd)                 0.5+0.00...1,'#f00'(overwrite pre)
0,     '#00f'            0, '#00f'                                                1, '#ff0'
Comment 2 Simon Hausmann 2010-03-22 15:18:18 PDT
Comment on attachment 51312 [details]
fix platformGradient


> +    std::sort(m_stops.begin(), m_stops.end(), stopColorSortPredicate);

I think you should use Gradient::sortStopsIfNecessary() here instead.

The rest looks good to me. r- for the above.
Comment 3 qi 2010-03-23 07:04:38 PDT
Created attachment 51422 [details]
use sortStopsIfNecessary

Use sortStopsIfNecessary instead of std:sort
Comment 4 WebKit Commit Bot 2010-03-23 09:34:22 PDT
Comment on attachment 51422 [details]
use sortStopsIfNecessary

Clearing flags on attachment: 51422

Committed r56398: <http://trac.webkit.org/changeset/56398>
Comment 5 WebKit Commit Bot 2010-03-23 09:34:28 PDT
All reviewed patches have been landed.  Closing bug.