[Qt] Mobile theme refinements
Created attachment 119629 [details] Patch As Kenneth rightfully pointed out, we should probably try to use the squircle shape for most of the controls in the mobile theme.
Comment on attachment 119629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119629&action=review > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:326 > + + (focused ? QLatin1String("-focused") : QString()); Usually string manipulation in the painting routine is a bad idea. It was one of the bottleneck in QGraphicsView back then. I recommend to try using the Key API of QPixmapCache (that is way faster). http://developer.qt.nokia.com/doc/qt-4.8/qpixmapcache.html This benchmark shows the improvements and implement a complicated use case such as pixmaps caching for our styles (and it is way faster). I'm sure we can use it here in a way. http://qt.gitorious.org/qt/qt/blobs/4.7/tests/benchmarks/gui/image/qpixmapcache/tst_qpixmapcache.cpp
(In reply to comment #2) > (From update of attachment 119629 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=119629&action=review > > > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:326 > > + + (focused ? QLatin1String("-focused") : QString()); > > Usually string manipulation in the painting routine is a bad idea. It was one of the bottleneck in QGraphicsView back then. > > I recommend to try using the Key API of QPixmapCache (that is way faster). http://developer.qt.nokia.com/doc/qt-4.8/qpixmapcache.html > > This benchmark shows the improvements and implement a complicated use case such as pixmaps caching for our styles (and it is way faster). I'm sure we can use it here in a way. > > http://qt.gitorious.org/qt/qt/blobs/4.7/tests/benchmarks/gui/image/qpixmapcache/tst_qpixmapcache.cpp I agree, but that is a separate patch - this is the way it was done by Luiz way back.
Comment on attachment 119629 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=119629&action=review > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:99 > + // Translating the painter has a tendency to mess with gradients. The comment is not clear. So you do thing differently here because of that, or something that should be fixed later? I think the comment could make that more clear > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:129 > + return 0; Can't a 0 mess things up? or is that considered invalid? > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:271 > + QPolygon upArrow, downArrow; > + upArrow << QPoint(0, arrowHeight) << QPoint(arrowHeight, 0) << QPoint(right, arrowHeight); > + downArrow << QPoint(0, bottomBaseline) << QPoint(arrowHeight, bottomBaseline + arrowHeight) > + << QPoint(right, bottomBaseline); Is this more effective? I think you can mention such changes in the changelog > Source/WebCore/platform/qt/RenderThemeQtMobile.cpp:680 > + } > + > + } else unneeded newline > Source/WebCore/platform/qt/RenderThemeQtMobile.h:113 > + QPixmap findLineEdit(const QSize&, bool focused) const; Maybe we should keep the names closer to the html form counterparts?
Created attachment 120024 [details] Patch
Comment on attachment 120024 [details] Patch Clearing flags on attachment: 120024 Committed r103404: <http://trac.webkit.org/changeset/103404>
All reviewed patches have been landed. Closing bug.