Summary: | Mismatched multiple box-shadows do not transition as expected | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Component: | CSS | Assignee: | Simon Fraser (smfr) <simon.fraser> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, bdakin, dglazkov, eric, hyatt, simon.fraser, webkit-ews, webkit.review.bot | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
The issue here is that shadows are processed in reverse order, and the last shadows have different types in the testcase. Created attachment 92193 [details]
Patch
Attachment 92193 [details] did not build on qt: Build output: http://queues.webkit.org/results/8551434 Attachment 92193 [details] did not pass chromium-ews: Output: http://queues.webkit.org/results/8567003 Created attachment 93245 [details]
Patch
Comment on attachment 93245 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93245&action=review r+ but please consider addressing at least some of the comments > Source/WebCore/page/animation/AnimationBase.cpp:330 > +static inline int shadowListLength(const ShadowData* shadow) Why not size_t? > Source/WebCore/page/animation/AnimationBase.cpp:394 > + if (fromLength == toLength || ((!fromLength || fromLength == 1) && (!toLength || toLength == 1))) { If you make shadowListLength return an unsigned type, you will be able to write “(!fromLength || fromLength == 1)” as “fromLength <= 1” > Source/WebCore/page/animation/AnimationBase.cpp:444 > + Vector<const ShadowData*> fromShadows(fromLength); > + Vector<const ShadowData*> toShadows(toLength); > + > + while (shadowA) { > + fromShadows.prepend(shadowA); > + shadowA = shadowA->next(); > + } > + > + while (shadowB) { > + toShadows.prepend(shadowB); > + shadowB = shadowB->next(); > + } This is an expensive way of doing things. You initialize fromShadows with fromLength 0 ShadowData pointers in it, then you prepend another fromLength pointers to that. You should do something like for (size_t i = fromLength; i >= 0; --i) { fromShadows[i - 1] = shadowA; shadowA = shadowA->next(); } And it would be better to give these vectors inline capacity to avoid heap allocation. http://trac.webkit.org/changeset/86351 might have broken GTK Linux 64-bit Debug The following tests are not passing: svg/custom/fill-opacity-update.svg |
Created attachment 92188 [details] Testcase A transition from one inset box-shadow to two shadows, one inset and one outset, doesn't work as expected. See testcase.