23816 2009-02-07 06:58:42 -0800 Clean up RenderSVGRoot.cpp 2009-02-09 12:55:59 -0800 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) Mac OS X 10.5 RESOLVED FIXED P2 Normal --- 1 rwlbuis webkit-unassigned oldest_to_newest 108820 0 rwlbuis 2009-02-07 06:58:42 -0800 RenderSVGRoot has some unneeded includes and unneeded if tests, clean that up. 108821 1 27445 rwlbuis 2009-02-07 07:00:28 -0800 Created attachment 27445 Cleanup patch Straightforward cleanup of RenderSVGRoot.cpp. Cheers, Rob. 108996 2 27445 eric 2009-02-09 11:10:51 -0800 Comment on attachment 27445 Cleanup patch Looks fine. You could even handle the hasSetContainerSize() case up front. That would result in more code, but might be cleaner. if (svg-> hasSetContainerSize()) { m_viewport = FloatRect(0, 0, width.value(svg), height.value(svg)); return; } Then the rest could be a terinary: w = (width.unitType() == LengthTypePercent) ? svg->relativeWidthValue() : width.value(svg); h = (height.unitType() == LengthTypePercent) ? svg->relativeHeightValue() : height.value(svg); m_viewport = FloatRect(0, 0, w, h); I'm not sure if that's cleaner or not though. Either way, looks great and doesn't need any further review. 109016 3 rwlbuis 2009-02-09 12:55:59 -0800 Hi Eric, (In reply to comment #2) > (From update of attachment 27445 [review]) > Looks fine. You could even handle the hasSetContainerSize() case up front. > That would result in more code, but might be cleaner. > > if (svg-> hasSetContainerSize()) { > m_viewport = FloatRect(0, 0, width.value(svg), height.value(svg)); > return; > } > > Then the rest could be a terinary: > > w = (width.unitType() == LengthTypePercent) ? svg->relativeWidthValue() : > width.value(svg); > h = (height.unitType() == LengthTypePercent) ? svg->relativeHeightValue() : > height.value(svg); > m_viewport = FloatRect(0, 0, w, h); > > I'm not sure if that's cleaner or not though. Either way, looks great and > doesn't need any further review. Thanks for the thumbs up, I did try to clean it up that way in my final commit. Cheers, Rob. 27445 2009-02-07 07:00:28 -0800 2009-02-09 11:10:51 -0800 Cleanup patch cleanupRenderSVGRoot.diff text/plain 4322 rwlbuis ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg ZmRiOTVmOC4uOTA0ZDQzZSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAwOS0wMi0wNyAgUm9iIEJ1aXMgIDxy d2xidWlzQGdtYWlsLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjM4MTYK KyAgICAgICAgQ2xlYW4gdXAgUmVuZGVyU1ZHUm9vdC5jcHAKKworICAgICAgICBDbGVhbiB1cCBS ZW5kZXJTVkdSb290LmNwcC4KKworICAgICAgICBXQVJOSU5HOiBOTyBURVNUIENBU0VTIEFEREVE IE9SIENIQU5HRUQKKworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJTVkdSb290LmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OlJlbmRlclNWR1Jvb3Q6OnBhaW50KToKKyAgICAgICAgKFdlYkNvcmU6 OlJlbmRlclNWR1Jvb3Q6OmNhbGNWaWV3cG9ydCk6CisKIDIwMDktMDItMDUgIFJvYiBCdWlzICA8 cndsYnVpc0BnbWFpbC5jb20+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CmRpZmYgLS1naXQgYS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJTVkdSb290LmNwcCBiL1dlYkNv cmUvcmVuZGVyaW5nL1JlbmRlclNWR1Jvb3QuY3BwCmluZGV4IGQwYmU1MzIuLmM0OTMzNTggMTAw NjQ0Ci0tLSBhL1dlYkNvcmUvcmVuZGVyaW5nL1JlbmRlclNWR1Jvb3QuY3BwCisrKyBiL1dlYkNv cmUvcmVuZGVyaW5nL1JlbmRlclNWR1Jvb3QuY3BwCkBAIC0yNywxNyArMjcsMTIgQEAKICNpbmNs dWRlICJSZW5kZXJTVkdSb290LmgiCiAKICNpbmNsdWRlICJHcmFwaGljc0NvbnRleHQuaCIKLSNp bmNsdWRlICJSZW5kZXJQYXRoLmgiCiAjaW5jbHVkZSAiUmVuZGVyU1ZHQ29udGFpbmVyLmgiCiAj aW5jbHVkZSAiUmVuZGVyVmlldy5oIgogI2luY2x1ZGUgIlNWR0xlbmd0aC5oIgogI2luY2x1ZGUg IlNWR1JlbmRlclN1cHBvcnQuaCIKLSNpbmNsdWRlICJTVkdSZXNvdXJjZUNsaXBwZXIuaCIKLSNp bmNsdWRlICJTVkdSZXNvdXJjZUZpbHRlci5oIgotI2luY2x1ZGUgIlNWR1Jlc291cmNlTWFza2Vy LmgiCiAjaW5jbHVkZSAiU1ZHU1ZHRWxlbWVudC5oIgogI2luY2x1ZGUgIlNWR1N0eWxlZEVsZW1l bnQuaCIKLSNpbmNsdWRlICJTVkdVUklSZWZlcmVuY2UuaCIKIAogdXNpbmcgbmFtZXNwYWNlIHN0 ZDsKIApAQCAtMTU0LDcgKzE0OSw2IEBAIHZvaWQgUmVuZGVyU1ZHUm9vdDo6cGFpbnQoUGFpbnRJ bmZvJiBwYWludEluZm8sIGludCBwYXJlbnRYLCBpbnQgcGFyZW50WSkKIAogICAgIGNhbGNWaWV3 cG9ydCgpOwogCi0gICAgU1ZHU1ZHRWxlbWVudCogc3ZnID0gc3RhdGljX2Nhc3Q8U1ZHU1ZHRWxl bWVudCo+KGVsZW1lbnQoKSk7CiAgICAgLy8gQSB2YWx1ZSBvZiB6ZXJvIGRpc2FibGVzIHJlbmRl cmluZyBvZiB0aGUgZWxlbWVudC4gCiAgICAgaWYgKHZpZXdwb3J0KCkud2lkdGgoKSA8PSAwLiB8 fCB2aWV3cG9ydCgpLmhlaWdodCgpIDw9IDAuKQogICAgICAgICByZXR1cm47CkBAIC0xODIsOCAr MTc2LDkgQEAgdm9pZCBSZW5kZXJTVkdSb290OjpwYWludChQYWludEluZm8mIHBhaW50SW5mbywg aW50IHBhcmVudFgsIGludCBwYXJlbnRZKQogCiAgICAgRmxvYXRSZWN0IGJvdW5kaW5nQm94ID0g cmVsYXRpdmVCQm94KHRydWUpOwogICAgIGlmIChjaGlsZFBhaW50SW5mby5waGFzZSA9PSBQYWlu dFBoYXNlRm9yZWdyb3VuZCkKLSAgICAgICAgcHJlcGFyZVRvUmVuZGVyU1ZHQ29udGVudCh0aGlz LCBjaGlsZFBhaW50SW5mbywgYm91bmRpbmdCb3gsIGZpbHRlcik7ICAgICAgICAKKyAgICAgICAg cHJlcGFyZVRvUmVuZGVyU1ZHQ29udGVudCh0aGlzLCBjaGlsZFBhaW50SW5mbywgYm91bmRpbmdC b3gsIGZpbHRlcik7CiAKKyAgICBTVkdTVkdFbGVtZW50KiBzdmcgPSBzdGF0aWNfY2FzdDxTVkdT VkdFbGVtZW50Kj4oZWxlbWVudCgpKTsKICAgICBjaGlsZFBhaW50SW5mby5jb250ZXh0LT5jb25j YXRDVE0oc3ZnLT52aWV3Qm94VG9WaWV3VHJhbnNmb3JtKHdpZHRoKCksIGhlaWdodCgpKSk7CiAg ICAgUmVuZGVyQm94OjpwYWludChjaGlsZFBhaW50SW5mbywgMCwgMCk7CiAKQEAgLTE5MSw3ICsx ODYsNyBAQCB2b2lkIFJlbmRlclNWR1Jvb3Q6OnBhaW50KFBhaW50SW5mbyYgcGFpbnRJbmZvLCBp bnQgcGFyZW50WCwgaW50IHBhcmVudFkpCiAgICAgICAgIGZpbmlzaFJlbmRlclNWR0NvbnRlbnQo dGhpcywgY2hpbGRQYWludEluZm8sIGJvdW5kaW5nQm94LCBmaWx0ZXIsIHBhaW50SW5mby5jb250 ZXh0KTsKIAogICAgIGNoaWxkUGFpbnRJbmZvLmNvbnRleHQtPnJlc3RvcmUoKTsKLSAgICAKKwog ICAgIGlmICgoY2hpbGRQYWludEluZm8ucGhhc2UgPT0gUGFpbnRQaGFzZU91dGxpbmUgfHwgY2hp bGRQYWludEluZm8ucGhhc2UgPT0gUGFpbnRQaGFzZVNlbGZPdXRsaW5lKSAmJiBzdHlsZSgpLT5v dXRsaW5lV2lkdGgoKSAmJiBzdHlsZSgpLT52aXNpYmlsaXR5KCkgPT0gVklTSUJMRSkKICAgICAg ICAgcGFpbnRPdXRsaW5lKGNoaWxkUGFpbnRJbmZvLmNvbnRleHQsIG1fYWJzb2x1dGVCb3VuZHMu eCgpLCBtX2Fic29sdXRlQm91bmRzLnkoKSwgbV9hYnNvbHV0ZUJvdW5kcy53aWR0aCgpLCBtX2Fi c29sdXRlQm91bmRzLmhlaWdodCgpLCBzdHlsZSgpKTsKIH0KQEAgLTIwMywyOCArMTk4LDI1IEBA IEZsb2F0UmVjdCBSZW5kZXJTVkdSb290Ojp2aWV3cG9ydCgpIGNvbnN0CiAKIHZvaWQgUmVuZGVy U1ZHUm9vdDo6Y2FsY1ZpZXdwb3J0KCkKIHsKLSAgICBTVkdFbGVtZW50KiBzdmdlbGVtID0gc3Rh dGljX2Nhc3Q8U1ZHRWxlbWVudCo+KGVsZW1lbnQoKSk7Ci0gICAgaWYgKHN2Z2VsZW0tPmhhc1Rh Z05hbWUoU1ZHTmFtZXM6OnN2Z1RhZykpIHsKLSAgICAgICAgU1ZHU1ZHRWxlbWVudCogc3ZnID0g c3RhdGljX2Nhc3Q8U1ZHU1ZHRWxlbWVudCo+KGVsZW1lbnQoKSk7CisgICAgU1ZHU1ZHRWxlbWVu dCogc3ZnID0gc3RhdGljX2Nhc3Q8U1ZHU1ZHRWxlbWVudCo+KGVsZW1lbnQoKSk7CiAKLSAgICAg ICAgaWYgKCFzZWxmTmVlZHNMYXlvdXQoKSAmJiAhc3ZnLT5oYXNSZWxhdGl2ZVZhbHVlcygpKQot ICAgICAgICAgICAgcmV0dXJuOworICAgIGlmICghc2VsZk5lZWRzTGF5b3V0KCkgJiYgIXN2Zy0+ aGFzUmVsYXRpdmVWYWx1ZXMoKSkKKyAgICAgICAgcmV0dXJuOwogCi0gICAgICAgIGZsb2F0IHcs IGg7Ci0gICAgICAgIFNWR0xlbmd0aCB3aWR0aCA9IHN2Zy0+d2lkdGgoKTsKLSAgICAgICAgaWYg KHdpZHRoLnVuaXRUeXBlKCkgPT0gTGVuZ3RoVHlwZVBlcmNlbnRhZ2UgJiYgc3ZnLT5oYXNTZXRD b250YWluZXJTaXplKCkpCi0gICAgICAgICAgICB3ID0gc3ZnLT5yZWxhdGl2ZVdpZHRoVmFsdWUo KTsKLSAgICAgICAgZWxzZQotICAgICAgICAgICAgdyA9IHdpZHRoLnZhbHVlKHN2Zyk7Ci0gICAg ICAgIAotICAgICAgICBTVkdMZW5ndGggaGVpZ2h0ID0gc3ZnLT5oZWlnaHQoKTsKLSAgICAgICAg aWYgKGhlaWdodC51bml0VHlwZSgpID09IExlbmd0aFR5cGVQZXJjZW50YWdlICYmIHN2Zy0+aGFz U2V0Q29udGFpbmVyU2l6ZSgpKQotICAgICAgICAgICAgaCA9IHN2Zy0+cmVsYXRpdmVIZWlnaHRW YWx1ZSgpOwotICAgICAgICBlbHNlCi0gICAgICAgICAgICBoID0gaGVpZ2h0LnZhbHVlKHN2Zyk7 CisgICAgZmxvYXQgdywgaDsKKyAgICBTVkdMZW5ndGggd2lkdGggPSBzdmctPndpZHRoKCk7Cisg ICAgaWYgKHdpZHRoLnVuaXRUeXBlKCkgPT0gTGVuZ3RoVHlwZVBlcmNlbnRhZ2UgJiYgc3ZnLT5o YXNTZXRDb250YWluZXJTaXplKCkpCisgICAgICAgIHcgPSBzdmctPnJlbGF0aXZlV2lkdGhWYWx1 ZSgpOworICAgIGVsc2UKKyAgICAgICAgdyA9IHdpZHRoLnZhbHVlKHN2Zyk7CiAKLSAgICAgICAg bV92aWV3cG9ydCA9IEZsb2F0UmVjdCgwLCAwLCB3LCBoKTsKLSAgICB9CisgICAgU1ZHTGVuZ3Ro IGhlaWdodCA9IHN2Zy0+aGVpZ2h0KCk7CisgICAgaWYgKGhlaWdodC51bml0VHlwZSgpID09IExl bmd0aFR5cGVQZXJjZW50YWdlICYmIHN2Zy0+aGFzU2V0Q29udGFpbmVyU2l6ZSgpKQorICAgICAg ICBoID0gc3ZnLT5yZWxhdGl2ZUhlaWdodFZhbHVlKCk7CisgICAgZWxzZQorICAgICAgICBoID0g aGVpZ2h0LnZhbHVlKHN2Zyk7CisKKyAgICBtX3ZpZXdwb3J0ID0gRmxvYXRSZWN0KDAsIDAsIHcs IGgpOwogfQogCiBJbnRSZWN0IFJlbmRlclNWR1Jvb3Q6OmNsaXBwZWRPdmVyZmxvd1JlY3RGb3JS ZXBhaW50KFJlbmRlckJveE1vZGVsT2JqZWN0KiByZXBhaW50Q29udGFpbmVyKQo=