Bug 51208 - [Qt] GraphicsContext should respect QWebView render hints
Summary: [Qt] GraphicsContext should respect QWebView render hints
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Ariya Hidayat
URL:
Keywords: HTML5, Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-12-16 12:47 PST by Ariya Hidayat
Modified: 2011-04-08 07:33 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.17 KB, patch)
2010-12-16 15:33 PST, Ariya Hidayat
no flags Details | Formatted Diff | Diff
[Qt] GraphicsContext should respect QWebView render hints (2.34 KB, patch)
2010-12-17 08:15 PST, Ariya Hidayat
no flags Details | Formatted Diff | Diff
2.1.x branch - patch to fix wrong cherry-pick (2.36 KB, patch)
2011-04-08 06:46 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff
2.1.x branch - patch to fix wrong cherry-pick, v2 (2.39 KB, patch)
2011-04-08 07:05 PDT, Caio Marcelo de Oliveira Filho
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ariya Hidayat 2010-12-16 12:47:22 PST
Because http://trac.webkit.org/changeset/62762, the render hint SmoothPixmapTransform is always set, regardless what is configured via QWebView::setRenderHint(s).
Comment 1 Ariya Hidayat 2010-12-16 15:33:43 PST
Created attachment 76820 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2010-12-16 16:21:26 PST
What about QGraphicsWebView and our WebKit2 port?
Comment 3 Ariya Hidayat 2010-12-16 16:50:30 PST
> What about QGraphicsWebView and our WebKit2 port?

For QGraphicsWebView, either the containing QWebView has to set the render hints properly (to get smooth pixmap) or we automatically set smooth pixmap in the paint function. But the latter is wrong, cause again it's clobbering the original painter's render hint.

For WebKit2, I'm not sure since I'm not familiar with it. I assume BackingStore::createGraphicsContext() is good place to insert additional set of smooth pixmap transform render hint. Are there any other places where GraphicsContext is created?
Comment 4 Andreas Kling 2010-12-16 17:32:38 PST
Comment on attachment 76820 [details]
Patch

r=me

For QGraphicsWebView we will (and should) just follow whatever we get from the view (set via QGraphicsView::setRenderHint())
Comment 5 Ariya Hidayat 2010-12-16 17:45:03 PST
Comment on attachment 76820 [details]
Patch

Clearing flags on attachment: 76820

Committed r74220: <http://trac.webkit.org/changeset/74220>
Comment 6 Ariya Hidayat 2010-12-16 17:45:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Csaba Osztrogonác 2010-12-17 05:46:38 PST
Reopen, because a Qt API test fails after this patch:

FAIL!  : tst_QWebView::renderHints() '!(webView.renderHints() & QPainter::SmoothPixmapTransform)' returned FALSE. ()
   Loc: [/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/WebKit/qt/tests/qwebview/tst_qwebview.cpp(85)]

Could you check it, please?
Comment 8 Ariya Hidayat 2010-12-17 08:15:47 PST
Created attachment 76883 [details]
[Qt] GraphicsContext should respect QWebView render hints
Comment 9 Antonio Gomes 2010-12-17 08:23:50 PST
Comment on attachment 76883 [details]
[Qt] GraphicsContext should respect QWebView render hints

I trust you here.

r=me
Comment 10 Ariya Hidayat 2010-12-17 08:27:08 PST
Comment on attachment 76883 [details]
[Qt] GraphicsContext should respect QWebView render hints

Clearing flags on attachment: 76883

Committed r74271: <http://trac.webkit.org/changeset/74271>
Comment 11 Ariya Hidayat 2010-12-17 08:27:14 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Caio Marcelo de Oliveira Filho 2011-03-28 09:17:12 PDT
Revision r74220 cherry-picked into qtwebkit-2.1.x with commit dcbf037442a531c9da18a879f91bedfe141aaea0 <http://gitorious.org/webkit/qtwebkit/commit/dcbf037442a531c9da18a879f91bedfe141aaea0>

Revision r74271 cherry-picked into qtwebkit-2.1.x with commit 2927c9d24ce15010eece90504cc0e1fc21e851e2 <http://gitorious.org/webkit/qtwebkit/commit/2927c9d24ce15010eece90504cc0e1fc21e851e2>
Comment 13 Fabrizio 2011-04-08 05:34:48 PDT
I am not so sure r74220 has been cherrypicked to 2.1.x.  We are still seeing test cases related to this failing on Symbian with latest 2.1.x.

Inside GraphicsContextPlatformPrivate::GrahicsContextPlatformPrivate() from GraphicsContextQt.cpp in 2.1.x, I do not see 'painter->setRenderHint(QPainter::Antialiasing,true)' getting called for all cases.  It's only getting called if platform != Symbian, but we actually need it called for all platforms, as in trunk.

Can we get this change to 2.1.x?
Comment 14 Caio Marcelo de Oliveira Filho 2011-04-08 06:46:52 PDT
Created attachment 88812 [details]
2.1.x branch - patch to fix wrong cherry-pick

Patch to fix mistake during the cherry-pick of r74220 in branch qtwebkit-2.1.x.
Comment 15 Caio Marcelo de Oliveira Filho 2011-04-08 06:51:34 PDT
For the record, from IRC:

<cmarcelo> lgombos: kenneth_: rubberstamp for fixing a mistake during cherry-pick (branch 2.1.x). it's the third patch in https://bugs.webkit.org/show_bug.cgi?id=51208  can one of you take a look?
<lgombos> cmarcelo: ok by me
Comment 16 Caio Marcelo de Oliveira Filho 2011-04-08 07:05:36 PDT
Created attachment 88818 [details]
2.1.x branch - patch to fix wrong cherry-pick, v2

Fix dumb mistake, 'imageInterpolationQuality' was missing.
Comment 17 Caio Marcelo de Oliveira Filho 2011-04-08 07:11:36 PDT
Fix in commit f45d42188432a78331e9fa5733863752ea81f64e of qtwebkit-2.1.x branch. Thanks for spotting this Fabrizio.
Comment 18 Fabrizio 2011-04-08 07:33:52 PDT
No problem Caio, thanks for your help.