Bug 55675 - Changing svg transform attribute to the same value triggers repaint
Summary: Changing svg transform attribute to the same value triggers repaint
Status: RESOLVED WONTFIX
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Patrick R. Gansterer
URL:
Keywords:
Depends on: 55829 55771
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-03 07:42 PST by Patrick R. Gansterer
Modified: 2014-05-12 06:23 PDT (History)
11 users (show)

See Also:


Attachments
Patch (3.12 KB, patch)
2011-03-04 10:07 PST, Patrick R. Gansterer
aroben: review-
Details | Formatted Diff | Diff
Alternative Patch (depends on bug 55829) (1.31 KB, patch)
2011-03-05 15:12 PST, Patrick R. Gansterer
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick R. Gansterer 2011-03-03 07:42:21 PST
When you have <rect fill="red" transform="scale(2)" /> and set the _fill_ attribute to "red" again it doesn't trigger a repaint.
If you set the _transform_ attribute of the rect to "scale(2)" again, it requests a repaint.
Comment 1 Patrick R. Gansterer 2011-03-04 10:07:56 PST
Created attachment 84772 [details]
Patch
Comment 2 Nikolas Zimmermann 2011-03-05 04:31:58 PST
Comment on attachment 84772 [details]
Patch

Are you sure it's worth the gain for the common-case, where the attribute indeed changed to a new value? Looks like an expensive test for a corner case...
Comment 3 Patrick R. Gansterer 2011-03-05 15:12:14 PST
Created attachment 84879 [details]
Alternative Patch (depends on bug 55829)

(In reply to comment #2)
> Are you sure it's worth the gain for the common-case, where the attribute indeed changed to a new value? Looks like an expensive test for a corner case...
I'm not 100% happy with the patch either, but I was my first try to fix this problem. I also added a a change comparison in the setAttribute directly, but many unit LayoutTest failed. This patch disables the whole svgAttributeChanged method, if the attribute stays the same but requires bug 55829.
Comment 4 WebKit Review Bot 2011-03-05 15:16:39 PST
Attachment 84879 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8105082
Comment 5 Build Bot 2011-03-05 15:34:37 PST
Attachment 84879 [details] did not build on win:
Build output: http://queues.webkit.org/results/8101158
Comment 6 Collabora GTK+ EWS bot 2011-03-05 16:01:26 PST
Attachment 84879 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/8106078
Comment 7 Early Warning System Bot 2011-03-05 16:05:48 PST
Attachment 84879 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8098361
Comment 8 WebKit Review Bot 2011-03-05 16:46:30 PST
Attachment 84879 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8103166
Comment 9 WebKit Review Bot 2011-03-05 17:11:46 PST
Attachment 84879 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8105104
Comment 10 Eric Seidel (no email) 2011-04-10 16:22:48 PDT
svgAttributeChanged is kinda a disaster anyway.  I can't remember why it's needed.
Comment 11 Nikolas Zimmermann 2011-04-11 08:01:08 PDT
(In reply to comment #10)
> svgAttributeChanged is kinda a disaster anyway.  I can't remember why it's needed.
You're saying this since ages, it's not a disaster in anyway, but just fine.
I explained several times why it's needed (unify SVG DOM _and_ XML DOM changes).
Comment 12 Adam Roben (:aroben) 2011-04-26 15:38:57 PDT
Comment on attachment 84772 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=84772&action=review

I don't understand why the code was moved from svgAttributeChanged to parseMappedAttribute. Your ChangeLog should probably explain this.

> Source/WebCore/ChangeLog:5
> +        Chaning svg transform attribute to the same value triggers repaint

Typo: Chaning
Comment 13 Adam Roben (:aroben) 2011-04-26 16:09:33 PDT
Comment on attachment 84879 [details]
Alternative Patch (depends on bug 55829)

View in context: https://bugs.webkit.org/attachment.cgi?id=84879&action=review

> Source/WebCore/svg/SVGElement.cpp:386
> -    if (attr->name() != HTMLNames::styleAttr)
> +    if (type != ChangedAttributeToSameValue && attr->name() != HTMLNames::styleAttr)

Does HTMLElement or Element do this?
Comment 14 Eric Seidel (no email) 2012-01-30 15:02:51 PST
Just noticed this nearly 12-month-old bug.  Nico: Thoughts on this change?  You're much more familiar with SVG these days than I.
Comment 15 Eric Seidel (no email) 2012-03-01 13:53:40 PST
Comment on attachment 84879 [details]
Alternative Patch (depends on bug 55829)

With no response, and no answer to Adam's question, marking this r-.  Feel free to mark this r? again with questions answered.
Comment 16 Dirk Schulze 2014-05-12 06:23:44 PDT
We do no longer have AttributeChangedType. Transforms should be handled by CSS in the future anyway.