49915 2010-11-22 09:21:13 -0800 [Qt] QWebFramePrivate::renderRelativeCoords() calls QPainter::save/restore more than necessary 2011-01-24 08:46:54 -0800 1 1 1 Unclassified WebKit WebKit Qt 528+ (Nightly build) All All RESOLVED FIXED Qt, QtTriaged P2 Normal --- 1 kling rhodovan.u-szeged abarth eric webkit.review.bot oldest_to_newest 312374 0 kling 2010-11-22 09:21:13 -0800 Find a way to minimize the number of save/restore calls. 338972 1 79917 rhodovan.u-szeged 2011-01-24 06:19:46 -0800 Created attachment 79917 Patch 338973 2 79917 kling 2011-01-24 06:24:08 -0800 Comment on attachment 79917 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79917&action=review > Source/WebKit/qt/ChangeLog:9 > + In the first loop of renderRelativeCoords() the call of QPainter::save/restore is useless, because > + the only change is on it's context which is restored. This sounds a bit strange, I would say something like "..., because the context is saved/restored within the loop." > Source/WebKit/qt/ChangeLog:10 > + In the second loop their calling is also avoidable by using invert translation on context. s/their calling is/the calls are/ s/invert/inverse/ > Source/WebKit/qt/Api/qwebframe.cpp:408 > view->paintScrollbars(context, rect); Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state? 338982 3 rhodovan.u-szeged 2011-01-24 06:49:11 -0800 (In reply to comment #2) > (From update of attachment 79917 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=79917&action=review > > > Source/WebKit/qt/ChangeLog:9 > > + In the first loop of renderRelativeCoords() the call of QPainter::save/restore is useless, because > > + the only change is on it's context which is restored. > > This sounds a bit strange, I would say something like "..., because the context is saved/restored within the loop." > > > Source/WebKit/qt/ChangeLog:10 > > + In the second loop their calling is also avoidable by using invert translation on context. > > s/their calling is/the calls are/ > s/invert/inverse/ Uppppdated... > > Source/WebKit/qt/Api/qwebframe.cpp:408 > > view->paintScrollbars(context, rect); > > Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state? No, GC is absolutely undamaged :) 338983 4 79917 kling 2011-01-24 06:53:01 -0800 Comment on attachment 79917 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=79917&action=review >>> Source/WebKit/qt/Api/qwebframe.cpp:408 >>> view->paintScrollbars(context, rect); >> >> Are you sure that FrameView::paintScrollbars() doesn't taint the GraphicsContext state? > > No, GC is absolutely undamaged :) Awesomecake! r=me 338987 5 rhodovan.u-szeged 2011-01-24 07:03:04 -0800 Closing bug. This is landed in <http://trac.webkit.org/changeset/76516> 339042 6 webkit.review.bot 2011-01-24 08:46:54 -0800 http://trac.webkit.org/changeset/76516 might have broken GTK Linux 32-bit Debug The following tests are not passing: editing/selection/extend-selection-bidi.html 79917 2011-01-24 06:19:46 -0800 2011-01-24 07:03:48 -0800 Patch save_restore.patch text/plain 2289 rhodovan.u-szeged SW5kZXg6IFNvdXJjZS9XZWJLaXQvcXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJLaXQvcXQvQ2hhbmdlTG9nCShyZXZpc2lvbiA3NjUwOCkKKysrIFNvdXJjZS9XZWJLaXQvcXQv Q2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDEtMjQgIFJl bmF0YSBIb2RvdmFuICA8cmVuaUB3ZWJraXQub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gUVdlYkZyYW1lUHJpdmF0ZTo6cmVuZGVyUmVs YXRpdmVDb29yZHMoKSBjYWxscyBRUGFpbnRlcjo6c2F2ZS9yZXN0b3JlIG1vcmUgdGhhbiBuZWNl c3NhcnkKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ5 OTE1CisKKyAgICAgICAgSW4gdGhlIGZpcnN0IGxvb3Agb2YgcmVuZGVyUmVsYXRpdmVDb29yZHMo KSB0aGUgY2FsbCBvZiBRUGFpbnRlcjo6c2F2ZS9yZXN0b3JlIGlzIHVzZWxlc3MsIGJlY2F1c2UK KyAgICAgICAgdGhlIG9ubHkgY2hhbmdlIGlzIG9uIGl0J3MgY29udGV4dCB3aGljaCBpcyByZXN0 b3JlZC4KKyAgICAgICAgSW4gdGhlIHNlY29uZCBsb29wIHRoZWlyIGNhbGxpbmcgaXMgYWxzbyBh dm9pZGFibGUgYnkgdXNpbmcgaW52ZXJ0IHRyYW5zbGF0aW9uIG9uIGNvbnRleHQuCisKKyAgICAg ICAgKiBBcGkvcXdlYmZyYW1lLmNwcDoKKyAgICAgICAgKFFXZWJGcmFtZVByaXZhdGU6OnJlbmRl clJlbGF0aXZlQ29vcmRzKToKKwogMjAxMS0wMS0yNCAgQW5kcmFzIEJlY3NpICA8YWJlY3NpQHdl YmtpdC5vcmc+CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgQ3NhYmEgT3N6dHJvZ29uw6FjLgpJbmRl eDogU291cmNlL1dlYktpdC9xdC9BcGkvcXdlYmZyYW1lLmNwcAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3Vy Y2UvV2ViS2l0L3F0L0FwaS9xd2ViZnJhbWUuY3BwCShyZXZpc2lvbiA3NjUwNSkKKysrIFNvdXJj ZS9XZWJLaXQvcXQvQXBpL3F3ZWJmcmFtZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTM1Nyw3ICsz NTcsNiBAQCB2b2lkIFFXZWJGcmFtZVByaXZhdGU6OnJlbmRlclJlbGF0aXZlQ29vCiAgICAgdmll dy0+dXBkYXRlTGF5b3V0QW5kU3R5bGVJZk5lZWRlZFJlY3Vyc2l2ZSgpOwogCiAgICAgaWYgKGxh eWVyICYgUVdlYkZyYW1lOjpDb250ZW50c0xheWVyKSB7Ci0gICAgICAgIHBhaW50ZXItPnNhdmUo KTsKICAgICAgICAgZm9yIChpbnQgaSA9IDA7IGkgPCB2ZWN0b3Iuc2l6ZSgpOyArK2kpIHsKICAg ICAgICAgICAgIGNvbnN0IFFSZWN0JiBjbGlwUmVjdCA9IHZlY3Rvci5hdChpKTsKIApAQCAtMzgz LDcgKzM4Miw2IEBAIHZvaWQgUVdlYkZyYW1lUHJpdmF0ZTo6cmVuZGVyUmVsYXRpdmVDb28KIAog ICAgICAgICAgICAgY29udGV4dC0+cmVzdG9yZSgpOwogICAgICAgICB9Ci0gICAgICAgIHBhaW50 ZXItPnJlc3RvcmUoKTsKICNpZiBVU0UoQUNDRUxFUkFURURfQ09NUE9TSVRJTkcpICYmIFVTRShU RVhUVVJFX01BUFBFUikKICAgICAgICAgcmVuZGVyQ29tcG9zaXRlZExheWVycyhjb250ZXh0LCBJ bnRSZWN0KGNsaXAuYm91bmRpbmdSZWN0KCkpKTsKICNlbmRpZgpAQCAtNDAzLDE1ICs0MDEsMTMg QEAgdm9pZCBRV2ViRnJhbWVQcml2YXRlOjpyZW5kZXJSZWxhdGl2ZUNvbwogICAgICAgICAgICAg aWYgKGxheWVyICYgUVdlYkZyYW1lOjpTY3JvbGxCYXJMYXllcgogICAgICAgICAgICAgICAgICYm ICF2aWV3LT5zY3JvbGxiYXJzU3VwcHJlc3NlZCgpCiAgICAgICAgICAgICAgICAgJiYgKHZpZXct Pmhvcml6b250YWxTY3JvbGxiYXIoKSB8fCB2aWV3LT52ZXJ0aWNhbFNjcm9sbGJhcigpKSkgewot ICAgICAgICAgICAgICAgIGNvbnRleHQtPnNhdmUoKTsKLQogICAgICAgICAgICAgICAgIFFSZWN0 IHJlY3QgPSBpbnRlcnNlY3RlZFJlY3Q7CiAgICAgICAgICAgICAgICAgY29udGV4dC0+dHJhbnNs YXRlKHgsIHkpOwogICAgICAgICAgICAgICAgIHJlY3QudHJhbnNsYXRlKC14LCAteSk7CiAKICAg ICAgICAgICAgICAgICB2aWV3LT5wYWludFNjcm9sbGJhcnMoY29udGV4dCwgcmVjdCk7CiAKLSAg ICAgICAgICAgICAgICBjb250ZXh0LT5yZXN0b3JlKCk7CisgICAgICAgICAgICAgICAgY29udGV4 dC0+dHJhbnNsYXRlKC14LCAteSk7CiAgICAgICAgICAgICB9CiAKICNpZiBFTkFCTEUoUEFOX1ND Uk9MTElORykK