Summary: | SVGRenderContainer forces too many kids to relayout | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | SVG | Assignee: | Nikolas Zimmermann <zimmermann> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | hyatt, jamesr, krit, zimmermann | ||||||
Priority: | P2 | ||||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Bug Depends on: | 41566 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2007-10-06 00:21:31 PDT
FYI, the "hasRelativeValues()" (broken) optimization was removed as part of fixing bug 15388. Because all renderers cache their absolute bounds, any time your bounds origin, or transform (or a parent's bounds origin or transform) changes *all* kids need to relayout. However, any time just bounds size changes (or parents bounds size changes) only relative-sized kids need to relayout. (The parent who's bounds size changed will invalidate the proper region for repaint.) After fixing our localTransform handling to make the renders pull (soon to land), we now know when the local transform has changed, but that information isn't actually very useful unless we stash it away in the LayoutState so that our container kids can use it to decide what type of layout to do. Finally working on this issue! First results are very promising, saves a lot of unneeded layouts. Created attachment 60535 [details]
Initial patch
Not marked for review yet, missing a changelog so far.
Created attachment 60547 [details]
Updated patch
This time including a ChangeLog, have fun! :-) Be sure to try Lively Kernel / animation examples once this lands.
(In reply to comment #5) > Created an attachment (id=60547) [details] > Updated patch > > This time including a ChangeLog, have fun! :-) Be sure to try Lively Kernel / animation examples once this lands. Great Patch Niko. Just a question about the LayoutTest. What causes the change? SVGRoot becomes 15px bigger, but I don't see the reason?!? (In reply to comment #6) > (In reply to comment #5) > > Created an attachment (id=60547) [details] [details] > > Updated patch > > > > This time including a ChangeLog, have fun! :-) Be sure to try Lively Kernel / animation examples once this lands. > > Great Patch Niko. Just a question about the LayoutTest. What causes the change? SVGRoot becomes 15px bigger, but I don't see the reason?!? I don't know why it had the wrong size before. If you look at the enclosing box, the new width is the right one. Not sure why it was wrong before - must be related to some premature optimizations, that calcViewport() was not called, when selfNeedsLayout was false (when not using relative lengths). Something like that, anyhow it's a progression. Comment on attachment 60547 [details]
Updated patch
Great patch! We discussed the failing test on IRC. Please add the explaination to the changelog as well. r=me.
|