47767 2010-10-16 06:00:22 -0700 [Qt] All widgets are rendered incorrectly when rendered through a cache 2010-10-22 03:00:37 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) All All RESOLVED FIXED Qt P1 Major --- 1 benjamin benjamin benjamin kenneth noam oldest_to_newest 295128 0 70952 benjamin 2010-10-16 06:00:22 -0700 Created attachment 70952 Sample widget showing the problem When QtWebKit render widgets on a pixmap instead of a QWidget, the widgets are rendered without a correct QStyleOption. This makes the widgets looks incorrect in many cases. For example, all widgets are rendered with a disabled state (with accelerated compositing or with tiling). The problem is that we initialize the QStyleOption from the widget the view is rendered on. This widget is obtained from the graphics context: QPaintDevice* dev = 0; if (painter) dev = painter->device(); if (dev && dev->devType() == QInternal::Widget) widget = static_cast<QWidget*>(dev); This is broken with caches because the type is pixmap, not widget. We could fix the issue by either: 1) introduce a target widget in PaintInfo 2) initialize the QStyleOption with sensible default value There is the use case where 2 graphics view show the same scene, one graphics view is enabled, the second is disabled. This can only be solved by (1) + flushing all caches all the time when there is more than one view. I am in favor of using (2) even if it is incorrect. This means the widgets would be rendered as active even if the QGraphicsView is in disabled state. I prefer this approach because I don't see a reasonable way to make it works when two graphics view show the same scene. The resulting bug would be that widgets always appear enabled, even when rendered through a disabled QGraphicsView. Input welcome :) 295135 1 kenneth 2010-10-16 08:14:24 -0700 (2) seems very fine with me. We really cannot support various views in a nice fashion as WebKit was not designed with this in mind, and it already breaks with Windowed plugins etc. (Just look at our PageClient implementations :( ) 295146 2 noam 2010-10-16 09:34:32 -0700 What is the user story for this? 295148 3 noam 2010-10-16 09:40:12 -0700 (In reply to comment #2) > What is the user story for this? I'm in favor of (2) as well, just hard to me to understand why this is a big problem - having a good user story for this would let us understand the right track. 295149 4 kenneth 2010-10-16 09:42:48 -0700 I guess it is the following: With QWebView the QStyle comes from the active QWebView, when using tiling etc, we paint to images, thus they have no access to an active QWidget, thus they get wrong style info and paint wrongly. 297420 5 71437 benjamin 2010-10-21 08:02:59 -0700 Created attachment 71437 Patch 297591 6 71437 hausmann 2010-10-21 13:11:30 -0700 Comment on attachment 71437 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71437&action=review > WebCore/platform/qt/RenderThemeQt.cpp:92 > + if (widget) > + option.initFrom(widget); > + else > + /* > + If a widget is not directly available for rendering, we fallback to default > + value for an active widget. > + */ > + option.state = QStyle::State_Active | QStyle::State_Enabled; From the style guide: "One-line control clauses should not use braces unless comments are included or a single statement spans multiple lines." So curly braces are needed here. The rest of the patch looks great! r+, but please this style buglet before landing :) 297877 7 benjamin 2010-10-22 03:00:37 -0700 Committed r70297: <http://trac.webkit.org/changeset/70297> 70952 2010-10-16 06:00:22 -0700 2010-10-16 06:00:22 -0700 Sample widget showing the problem range.html text/html 21 benjamin PGlucHV0IHR5cGU9InJhbmdlIj4K 71437 2010-10-21 08:02:59 -0700 2010-10-21 13:11:30 -0700 Patch 47767.patch text/plain 6627 benjamin ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MWE3OWI3Yy4uMWY2MmQ4MiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMiBAQAorMjAxMC0xMC0yMSAgQmVuamFtaW4gUG91 bGFpbiAgPGJlbmphbWluLnBvdWxhaW5Abm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gQWxsIHdpZGdldHMgYXJlIHJlbmRlcmVk IGluY29ycmVjdGx5IHdoZW4gcmVuZGVyZWQgdGhyb3VnaCBhIGNhY2hlCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00Nzc2NworCisgICAgICAgIFdoZW4g YSB3aWRnZXQgaXMgbm90IGF2YWlsYWJsZSB0byB0aGUgUmVuZGVyVGhlbWUsIGRlZmF1bHQKKyAg ICAgICAgdmFsdWUgYXJlIHVzZWQgZm9yIHRoZSBzdGF0ZS4KKworICAgICAgICAqIHBsYXRmb3Jt L3F0L1JlbmRlclRoZW1lUXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6aW5pdFN0eWxlT3B0aW9u KToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRlclRoZW1lUXQ6OnBhaW50QnV0dG9uKToKKyAgICAg ICAgKFdlYkNvcmU6OlJlbmRlclRoZW1lUXQ6OnBhaW50VGV4dEZpZWxkKToKKyAgICAgICAgKFdl YkNvcmU6OlJlbmRlclRoZW1lUXQ6OnBhaW50TWVudUxpc3QpOgorICAgICAgICAoV2ViQ29yZTo6 UmVuZGVyVGhlbWVRdDo6cGFpbnRNZW51TGlzdEJ1dHRvbik6CisgICAgICAgIChXZWJDb3JlOjpS ZW5kZXJUaGVtZVF0OjpwYWludFByb2dyZXNzQmFyKToKKyAgICAgICAgKFdlYkNvcmU6OlJlbmRl clRoZW1lUXQ6OnBhaW50U2xpZGVyVHJhY2spOgorCiAyMDEwLTEwLTIxICBTaGVyaWZmIEJvdCAg PHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNvbT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xs aW5nIG91dCByNzAyMjUuCmRpZmYgLS1naXQgYS9XZWJDb3JlL3BsYXRmb3JtL3F0L1JlbmRlclRo ZW1lUXQuY3BwIGIvV2ViQ29yZS9wbGF0Zm9ybS9xdC9SZW5kZXJUaGVtZVF0LmNwcAppbmRleCA1 MGI1ZGU2Li45MjQ3ZTVlIDEwMDY0NAotLS0gYS9XZWJDb3JlL3BsYXRmb3JtL3F0L1JlbmRlclRo ZW1lUXQuY3BwCisrKyBiL1dlYkNvcmUvcGxhdGZvcm0vcXQvUmVuZGVyVGhlbWVRdC5jcHAKQEAg LTgwLDYgKzgwLDE4IEBAIG5hbWVzcGFjZSBXZWJDb3JlIHsKIAogdXNpbmcgbmFtZXNwYWNlIEhU TUxOYW1lczsKIAoraW5saW5lIHN0YXRpYyB2b2lkIGluaXRTdHlsZU9wdGlvbihRV2lkZ2V0ICp3 aWRnZXQsIFFTdHlsZU9wdGlvbiYgb3B0aW9uKQoreworICAgIGlmICh3aWRnZXQpCisgICAgICAg IG9wdGlvbi5pbml0RnJvbSh3aWRnZXQpOworICAgIGVsc2UKKyAgICAgICAgLyoKKyAgICAgICAg ICBJZiBhIHdpZGdldCBpcyBub3QgZGlyZWN0bHkgYXZhaWxhYmxlIGZvciByZW5kZXJpbmcsIHdl IGZhbGxiYWNrIHRvIGRlZmF1bHQKKyAgICAgICAgICB2YWx1ZSBmb3IgYW4gYWN0aXZlIHdpZGdl dC4KKyAgICAgICAgICovCisgICAgICAgIG9wdGlvbi5zdGF0ZSA9IFFTdHlsZTo6U3RhdGVfQWN0 aXZlIHwgUVN0eWxlOjpTdGF0ZV9FbmFibGVkOworfQorCiAKIFN0eWxlUGFpbnRlcjo6U3R5bGVQ YWludGVyKFJlbmRlclRoZW1lUXQqIHRoZW1lLCBjb25zdCBQYWludEluZm8mIHBhaW50SW5mbykK IHsKQEAgLTUzOCw5ICs1NTAsNyBAQCBib29sIFJlbmRlclRoZW1lUXQ6OnBhaW50QnV0dG9uKFJl bmRlck9iamVjdCogbywgY29uc3QgUGFpbnRJbmZvJiBpLCBjb25zdCBJbnRSZQogICAgICAgIHJl dHVybiB0cnVlOwogCiAgICAgUVN0eWxlT3B0aW9uQnV0dG9uIG9wdGlvbjsKLSAgICBpZiAocC53 aWRnZXQpCi0gICAgICAgb3B0aW9uLmluaXRGcm9tKHAud2lkZ2V0KTsKLQorICAgIGluaXRTdHls ZU9wdGlvbihwLndpZGdldCwgb3B0aW9uKTsKICAgICBvcHRpb24ucmVjdCA9IHI7CiAgICAgb3B0 aW9uLnN0YXRlIHw9IFFTdHlsZTo6U3RhdGVfU21hbGw7CiAKQEAgLTU3MSw5ICs1ODEsNyBAQCBi b29sIFJlbmRlclRoZW1lUXQ6OnBhaW50VGV4dEZpZWxkKFJlbmRlck9iamVjdCogbywgY29uc3Qg UGFpbnRJbmZvJiBpLCBjb25zdCBJbgogICAgICAgICByZXR1cm4gdHJ1ZTsKIAogICAgIFFTdHls ZU9wdGlvbkZyYW1lVjIgcGFuZWw7Ci0gICAgaWYgKHAud2lkZ2V0KQotICAgICAgICBwYW5lbC5p bml0RnJvbShwLndpZGdldCk7Ci0KKyAgICBpbml0U3R5bGVPcHRpb24ocC53aWRnZXQsIHBhbmVs KTsKICAgICBwYW5lbC5yZWN0ID0gcjsKICAgICBwYW5lbC5saW5lV2lkdGggPSBmaW5kRnJhbWVM aW5lV2lkdGgocVN0eWxlKCkpOwogICAgIHBhbmVsLnN0YXRlIHw9IFFTdHlsZTo6U3RhdGVfU3Vu a2VuOwpAQCAtNjQwLDggKzY0OCw3IEBAIGJvb2wgUmVuZGVyVGhlbWVRdDo6cGFpbnRNZW51TGlz dChSZW5kZXJPYmplY3QqIG8sIGNvbnN0IFBhaW50SW5mbyYgaSwgY29uc3QgSW50CiAgICAgICAg IHJldHVybiB0cnVlOwogCiAgICAgUXRTdHlsZU9wdGlvbldlYkNvbWJvQm94IG9wdChvKTsKLSAg ICBpZiAocC53aWRnZXQpCi0gICAgICAgIG9wdC5pbml0RnJvbShwLndpZGdldCk7CisgICAgaW5p dFN0eWxlT3B0aW9uKHAud2lkZ2V0LCBvcHQpOwogICAgIGluaXRpYWxpemVDb21tb25RU3R5bGVP cHRpb25zKG9wdCwgbyk7CiAKICAgICBjb25zdCBRUG9pbnQgdG9wTGVmdCA9IHIudG9wTGVmdCgp OwpAQCAtNjg0LDggKzY5MSw3IEBAIGJvb2wgUmVuZGVyVGhlbWVRdDo6cGFpbnRNZW51TGlzdEJ1 dHRvbihSZW5kZXJPYmplY3QqIG8sIGNvbnN0IFBhaW50SW5mbyYgaSwKICAgICAgICAgcmV0dXJu IHRydWU7CiAKICAgICBRdFN0eWxlT3B0aW9uV2ViQ29tYm9Cb3ggb3B0aW9uKG8pOwotICAgIGlm IChwLndpZGdldCkKLSAgICAgICAgb3B0aW9uLmluaXRGcm9tKHAud2lkZ2V0KTsKKyAgICBpbml0 U3R5bGVPcHRpb24ocC53aWRnZXQsIG9wdGlvbik7CiAgICAgaW5pdGlhbGl6ZUNvbW1vblFTdHls ZU9wdGlvbnMob3B0aW9uLCBvKTsKICAgICBvcHRpb24ucmVjdCA9IHI7CiAKQEAgLTczNSw4ICs3 NDEsNyBAQCBib29sIFJlbmRlclRoZW1lUXQ6OnBhaW50UHJvZ3Jlc3NCYXIoUmVuZGVyT2JqZWN0 KiBvLCBjb25zdCBQYWludEluZm8mIHBpLCBjb25zdAogICAgICAgIHJldHVybiB0cnVlOwogCiAg ICAgUVN0eWxlT3B0aW9uUHJvZ3Jlc3NCYXJWMiBvcHRpb247Ci0gICAgaWYgKHAud2lkZ2V0KQot ICAgICAgIG9wdGlvbi5pbml0RnJvbShwLndpZGdldCk7CisgICAgaW5pdFN0eWxlT3B0aW9uKHAu d2lkZ2V0LCBvcHRpb24pOwogICAgIGluaXRpYWxpemVDb21tb25RU3R5bGVPcHRpb25zKG9wdGlv biwgbyk7CiAKICAgICBSZW5kZXJQcm9ncmVzcyogcmVuZGVyUHJvZ3Jlc3MgPSB0b1JlbmRlclBy b2dyZXNzKG8pOwpAQCAtNzc3LDggKzc4Miw3IEBAIGJvb2wgUmVuZGVyVGhlbWVRdDo6cGFpbnRT bGlkZXJUcmFjayhSZW5kZXJPYmplY3QqIG8sIGNvbnN0IFBhaW50SW5mbyYgcGksCiAgICAgICAg cmV0dXJuIHRydWU7CiAKICAgICBRU3R5bGVPcHRpb25TbGlkZXIgb3B0aW9uOwotICAgIGlmIChw LndpZGdldCkKLSAgICAgICBvcHRpb24uaW5pdEZyb20ocC53aWRnZXQpOworICAgIGluaXRTdHls ZU9wdGlvbihwLndpZGdldCwgb3B0aW9uKTsKICAgICBvcHRpb24uc3ViQ29udHJvbHMgPSBRU3R5 bGU6OlNDX1NsaWRlckdyb292ZSB8IFFTdHlsZTo6U0NfU2xpZGVySGFuZGxlOwogICAgIENvbnRy b2xQYXJ0IGFwcGVhcmFuY2UgPSBpbml0aWFsaXplQ29tbW9uUVN0eWxlT3B0aW9ucyhvcHRpb24s IG8pOwogCmRpZmYgLS1naXQgYS9XZWJLaXQvcXQvQ2hhbmdlTG9nIGIvV2ViS2l0L3F0L0NoYW5n ZUxvZwppbmRleCBjMWJmNjY0Li5hZWNmZTRiIDEwMDY0NAotLS0gYS9XZWJLaXQvcXQvQ2hhbmdl TG9nCisrKyBiL1dlYktpdC9xdC9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNSBAQAorMjAxMC0xMC0y MSAgQmVuamFtaW4gUG91bGFpbiAgPGJlbmphbWluLnBvdWxhaW5Abm9raWEuY29tPgorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFtRdF0gQWxsIHdpZGdl dHMgYXJlIHJlbmRlcmVkIGluY29ycmVjdGx5IHdoZW4gcmVuZGVyZWQgdGhyb3VnaCBhIGNhY2hl CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00Nzc2Nwor CisgICAgICAgIEFkZCBhIG5ldyB0ZXN0IGZvciByZW5kZXJpbmcgd2l0aCB0aWxpbmcuCisKKyAg ICAgICAgKiB0ZXN0cy9xZ3JhcGhpY3N3ZWJ2aWV3L3RzdF9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcDoK KyAgICAgICAgKHRzdF9RR3JhcGhpY3NXZWJWaWV3Ojp3aWRnZXRzUmVuZGVyaW5nVGhyb3VnaENh Y2hlKToKKwogMjAxMC0xMC0yMCAgTHVpeiBBZ29zdGluaSAgPGx1aXouYWdvc3RpbmlAb3BlbmJv c3NhLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBBbnRvbmlvIEdvbWVzLgpkaWZmIC0tZ2l0 IGEvV2ViS2l0L3F0L3Rlc3RzL3FncmFwaGljc3dlYnZpZXcvdHN0X3FncmFwaGljc3dlYnZpZXcu Y3BwIGIvV2ViS2l0L3F0L3Rlc3RzL3FncmFwaGljc3dlYnZpZXcvdHN0X3FncmFwaGljc3dlYnZp ZXcuY3BwCmluZGV4IGEwNGZmMTcuLjU2NzM0ODggMTAwNjQ0Ci0tLSBhL1dlYktpdC9xdC90ZXN0 cy9xZ3JhcGhpY3N3ZWJ2aWV3L3RzdF9xZ3JhcGhpY3N3ZWJ2aWV3LmNwcAorKysgYi9XZWJLaXQv cXQvdGVzdHMvcWdyYXBoaWNzd2Vidmlldy90c3RfcWdyYXBoaWNzd2Vidmlldy5jcHAKQEAgLTM1 LDYgKzM1LDcgQEAgcHJpdmF0ZSBzbG90czoKICAgICB2b2lkIG1pY3JvRm9jdXNDb29yZGluYXRl cygpOwogICAgIHZvaWQgZm9jdXNJbnB1dFR5cGVzKCk7CiAgICAgdm9pZCBjcmFzaE9uU2V0U2Nh bGVCZWZvcmVTZXRVcmwoKTsKKyAgICB2b2lkIHdpZGdldHNSZW5kZXJpbmdUaHJvdWdoQ2FjaGUo KTsKIH07CiAKIHZvaWQgdHN0X1FHcmFwaGljc1dlYlZpZXc6OnFncmFwaGljc3dlYnZpZXcoKQpA QCAtMTQwLDYgKzE0MSw0MSBAQCB2b2lkIHRzdF9RR3JhcGhpY3NXZWJWaWV3OjpjcmFzaE9uU2V0 U2NhbGVCZWZvcmVTZXRVcmwoKQogICAgIGRlbGV0ZSB3ZWJWaWV3OwogfQogCit2b2lkIHRzdF9R R3JhcGhpY3NXZWJWaWV3Ojp3aWRnZXRzUmVuZGVyaW5nVGhyb3VnaENhY2hlKCkKK3sKKyAgICAv LyBXaWRnZXRzIHNob3VsZCBiZSByZW5kZXJlZCB0aGUgc2FtZSB3YXkgd2l0aCBhbmQgd2l0aG91 dAorICAgIC8vIGludGVybWVkaWF0ZSBjYWNoZSAodGlsaW5nIGZvciBleGFtcGxlKS4KKyAgICAv LyBTZWUgYnVnIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00Nzc2NyB3 aGVyZQorICAgIC8vIHdpZGdldCBhcmUgcmVuZGVyZWQgYXMgZGlzYWJsZWQgd2hlbiBjYWNoaW5n IGlzIHVzaW5nLgorCisgICAgUUdyYXBoaWNzV2ViVmlldyogd2ViVmlldyA9IG5ldyBRR3JhcGhp Y3NXZWJWaWV3OworICAgIHdlYlZpZXctPnNldEh0bWwoUUxhdGluMVN0cmluZygiPGJvZHkgc3R5 bGU9XCJiYWNrZ3JvdW5kLWNvbG9yOiB3aGl0ZVwiPjxpbnB1dCB0eXBlPXJhbmdlPjwvaW5wdXQ+ PGlucHV0IHR5cGU9Y2hlY2tib3g+PC9pbnB1dD48aW5wdXQgdHlwZT1yYWRpbz48L2lucHV0Pjxp bnB1dCB0eXBlPWZpbGU+PC9pbnB1dD48L2JvZHk+IikpOworCisgICAgUUdyYXBoaWNzVmlldyB2 aWV3OworICAgIHZpZXcuc2hvdygpOworICAgIFFHcmFwaGljc1NjZW5lKiBzY2VuZSA9IG5ldyBR R3JhcGhpY3NTY2VuZSgmdmlldyk7CisgICAgdmlldy5zZXRTY2VuZShzY2VuZSk7CisgICAgc2Nl bmUtPmFkZEl0ZW0od2ViVmlldyk7CisgICAgdmlldy5zZXRHZW9tZXRyeShRUmVjdCgwLCAwLCA1 MDAsIDUwMCkpOworICAgIFFXaWRnZXQgKmNvbnN0IHdpZGdldCA9ICZ2aWV3OworICAgIFFUZXN0 OjpxV2FpdEZvcldpbmRvd1Nob3duKHdpZGdldCk7CisKKyAgICAvLyAxLiBSZWZlcmVuY2Ugd2l0 aG91dCB0aWxpbmcuCisgICAgd2ViVmlldy0+c2V0dGluZ3MoKS0+c2V0QXR0cmlidXRlKFFXZWJT ZXR0aW5nczo6VGlsZWRCYWNraW5nU3RvcmVFbmFibGVkLCBmYWxzZSk7CisgICAgUVBpeG1hcCBy ZWZlcmVuY2VQaXhtYXAodmlldy5zaXplKCkpOworICAgIHdpZGdldC0+cmVuZGVyKCZyZWZlcmVu Y2VQaXhtYXApOworCisgICAgLy8gMi4gV2l0aCB0aWxpbmcuCisgICAgd2ViVmlldy0+c2V0dGlu Z3MoKS0+c2V0QXR0cmlidXRlKFFXZWJTZXR0aW5nczo6VGlsZWRCYWNraW5nU3RvcmVFbmFibGVk LCB0cnVlKTsKKyAgICBRUGl4bWFwIHZpZXdXaXRoVGlsaW5nKHZpZXcuc2l6ZSgpKTsKKyAgICB3 aWRnZXQtPnJlbmRlcigmdmlld1dpdGhUaWxpbmcpOworICAgIFFBcHBsaWNhdGlvbjo6cHJvY2Vz c0V2ZW50cygpOworICAgIHZpZXdXaXRoVGlsaW5nLmZpbGwoKTsKKyAgICB3aWRnZXQtPnJlbmRl cigmdmlld1dpdGhUaWxpbmcpOworCisgICAgUUNPTVBBUkUocmVmZXJlbmNlUGl4bWFwLnRvSW1h Z2UoKSwgdmlld1dpdGhUaWxpbmcudG9JbWFnZSgpKTsKK30KKwogdm9pZCB0c3RfUUdyYXBoaWNz V2ViVmlldzo6bWljcm9Gb2N1c0Nvb3JkaW5hdGVzKCkKIHsKICAgICBRV2ViUGFnZSogcGFnZSA9 IG5ldyBRV2ViUGFnZTsK