Summary: | Logic to track whether elements are using relative lengths is incomplete | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Nikolas Zimmermann <zimmermann> | ||||||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | krit, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 15391 | ||||||||||
Attachments: |
|
Description
Nikolas Zimmermann
2010-07-03 01:17:04 PDT
Created attachment 60442 [details] Initial patch This prepares the actual patch to fix bug 15391. Gives a huge speed benefit, whenever elements with absolute sizes are used, that do not need to relayout anymore. The actual logic for that comes in the patch for bug 15391, this is just a preparation. Attachment 60442 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/svg/SVGStyledElement.cpp:371: Tab found; better to use spaces [whitespace/tab] [1]
Total errors found: 1 in 43 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60444 [details]
Updated patch
Oops, remove leftover tab.
Comment on attachment 60444 [details] Updated patch > { > SVGStyledTransformableElement::svgAttributeChanged(attrName); > > + bool isLengthAttribute = attrName == SVGNames::cxAttr > + || attrName == SVGNames::cyAttr > + || attrName == SVGNames::rAttr; > + > + if (isLengthAttribute) > + updateRelativeLengthsInformation(); > + > RenderPath* renderer = static_cast<RenderPath*>(this->renderer()); > if (!renderer) updateRelativeLengthsInformation is not in this patch. Do we need to call it, even if we don't have a renderer? I guess you want to update the information about relative values, even if a renderer was not created yet, to avoid a second layout, correct? > +void SVGStyledElement::updateRelativeLengthsInformation(bool, SVGStyledElement*) > +{ > + // FIXME: The actual code will land in a follow-up patch. > +} > + > } Please add a link to this bug, even if you provide a followup soon. It looks like you changed the behavior of the filterElement. Do we have dynamic update tests for filter? If not, can you add some? (In reply to comment #4) > (From update of attachment 60444 [details]) > > { > > SVGStyledTransformableElement::svgAttributeChanged(attrName); > > > > + bool isLengthAttribute = attrName == SVGNames::cxAttr > > + || attrName == SVGNames::cyAttr > > + || attrName == SVGNames::rAttr; > > + > > + if (isLengthAttribute) > > + updateRelativeLengthsInformation(); > > + > > RenderPath* renderer = static_cast<RenderPath*>(this->renderer()); > > if (!renderer) > > updateRelativeLengthsInformation is not in this patch. Do we need to call it, even if we don't have a renderer? I guess you want to update the information about relative values, even if a renderer was not created yet, to avoid a second layout, correct? Well, it's necessary to build this information while parsing, before renderers are available. As soon as renderers are attached, and layouted we need the relative length information. > > +void SVGStyledElement::updateRelativeLengthsInformation(bool, SVGStyledElement*) > > +{ > > + // FIXME: The actual code will land in a follow-up patch. > > +} > > + > > } > > Please add a link to this bug, even if you provide a followup soon. Will do. > > It looks like you changed the behavior of the filterElement. Do we have dynamic update tests for filter? If not, can you add some? I hope you could mail me your set? You said you've already created some. (In reply to comment #5) > I hope you could mail me your set? You said you've already created some. I'll search them. I changed the hard disk, so I can't promise that I still have them :-( Created attachment 60530 [details]
Updated patch v2
Comment on attachment 60530 [details]
Updated patch v2
r=me
|