Bug 51208

Summary: [Qt] GraphicsContext should respect QWebView render hints
Product: WebKit Reporter: Ariya Hidayat <ariya.hidayat>
Component: CanvasAssignee: Ariya Hidayat <ariya.hidayat>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, fabrizio.machado, kenneth, kling, mdelaney7, ossy, tonikitoo
Priority: P2 Keywords: HTML5, Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Patch
none
[Qt] GraphicsContext should respect QWebView render hints
none
2.1.x branch - patch to fix wrong cherry-pick
none
2.1.x branch - patch to fix wrong cherry-pick, v2 none

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.