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

qi
Reported 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.
Attachments
fix platformGradient (66.42 KB, patch)
2010-03-22 11:09 PDT, qi
hausmann: review-
use sortStopsIfNecessary (66.60 KB, patch)
2010-03-23 07:04 PDT, qi
no flags
qi
Comment 1 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'
Simon Hausmann
Comment 2 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.
qi
Comment 3 2010-03-23 07:04:38 PDT
Created attachment 51422 [details] use sortStopsIfNecessary Use sortStopsIfNecessary instead of std:sort
WebKit Commit Bot
Comment 4 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>
WebKit Commit Bot
Comment 5 2010-03-23 09:34:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.