Summary: | <canvas> gradient stops at identical offsets are applied in the wrong order | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pam Greene (IRC:pamg) <pam> | ||||||||||
Component: | Layout and Rendering | Assignee: | Pam Greene (IRC:pamg) <pam> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Minor | Keywords: | InRadar, PlatformOnly | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 523.x (Safari 3) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Windows XP | ||||||||||||
Attachments: |
|
Description
Pam Greene (IRC:pamg)
2007-06-22 15:53:28 PDT
Created attachment 15188 [details]
testcase
I can't assign this to myself, but I'm working on it. Thanks for the bug report, Pam! Could you post screenshots of the correct and incorrect renderings as well? Created attachment 15230 [details]
correct & incorrect screenshot snippets
Created attachment 15276 [details]
Add a sequence number to stops and use it in comparisons
Comment on attachment 15276 [details]
Add a sequence number to stops and use it in comparisons
I don't think qsort is guaranteed to be stable on Mac either, so seems like this code could be switched to use std::stable_sort like RenderLayer.cpp does for example.
Comment on attachment 15276 [details]
Add a sequence number to stops and use it in comparisons
Should just fix this to use the right sorting algorithm instead.
Comment on attachment 15276 [details]
Add a sequence number to stops and use it in comparisons
The test would be a bit cleaner if it added the canvas elements dynamically through the DOM instead of specifying them in the markup.
(In reply to comment #8) > (From update of attachment 15276 [details] [edit]) > Should just fix this to use the right sorting algorithm instead. One sorting algorithm you could use is std::stable_sort. Created attachment 15308 [details] Use std::stable_sort and create <canvas> dynamically > this code could be switched to use std::stable_sort like RenderLayer.cpp does > Should just fix this to use the right sorting algorithm instead. > One sorting algorithm you could use is std::stable_sort. Okay okay, I get the point. :-) Here's that, along with a test change to create the canvas elements dynamically. Comment on attachment 15308 [details]
Use std::stable_sort and create <canvas> dynamically
r=me
One small thing should be improved.
-static int compareStops(const void* a, const void* b)
+inline bool compareStops(const CanvasGradient::ColorStop &a, const CanvasGradient::ColorStop &b)
It's good for this to be inline, but it still needs to be internal linkage, so it should say "static inline", not just "inline".
Landed in r23912. |