Bug 35866 - [Qt] Enable accelerated compositing by default
Summary: [Qt] Enable accelerated compositing by default
Status: CLOSED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other All
: P2 Normal
Assignee: Simon Hausmann
URL:
Keywords: Qt
: 35522 (view as bug list)
Depends on: 36234 37381
Blocks: 38744
  Show dependency treegraph
 
Reported: 2010-03-08 07:39 PST by Simon Hausmann
Modified: 2010-05-14 08:42 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.04 KB, patch)
2010-03-08 07:39 PST, Simon Hausmann
no flags Details | Formatted Diff | Diff
enable AC, skip failing tests (3.39 KB, patch)
2010-03-16 14:43 PDT, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Hausmann 2010-03-08 07:39:10 PST
[Qt] Enable accelerated compositing by default
Comment 1 Simon Hausmann 2010-03-08 07:39:39 PST
Created attachment 50220 [details]
Patch
Comment 2 Simon Hausmann 2010-03-08 07:40:43 PST
Noam, could you also take a quick look at this patch and double check the "documentation" I've added?

Thanks :)
Comment 3 Noam Rosenthal 2010-03-08 10:49:26 PST
I'd also maybe add a word about 3D and video? without AC the perspective attribute doesn't render at all, and the video tag makes WebCore render for each frame. So there are some UIs that are only achievable with AC, regardless of performance.
Otherwise LGTM
Comment 4 Tor Arne Vestbø 2010-03-10 06:44:14 PST
Please follow the QtWebKit bug reporting guidelines when reporting bugs.

See http://trac.webkit.org/wiki/QtWebKitBugs

Specifically:

  - The 'QtWebKit' component should only be used for bugs/features in the
    public QtWebKit API layer, not to signify that the bug is specific to
    the Qt port of WebKit

      http://trac.webkit.org/wiki/QtWebKitBugs#Component

  - Add the keyword 'Qt' to signal that it's a Qt-related bug

      http://trac.webkit.org/wiki/QtWebKitBugs#Keywords
Comment 5 Kenneth Rohde Christiansen 2010-03-12 12:38:43 PST
Comment on attachment 50220 [details]
Patch

Looks good, but a follow up patch is needed to the QtLauncher.
Comment 6 WebKit Commit Bot 2010-03-13 01:21:30 PST
Comment on attachment 50220 [details]
Patch

Clearing flags on attachment: 50220

Committed r55955: <http://trac.webkit.org/changeset/55955>
Comment 7 WebKit Commit Bot 2010-03-13 01:21:34 PST
All reviewed patches have been landed.  Closing bug.
Comment 8 Csaba Osztrogonác 2010-03-13 10:14:31 PST
This patch broke 3 animation test, so I had to roll-out. (http://trac.webkit.org/changeset/55963)

detailed results:
http://build.webkit.org/results/Qt%20Linux%20Release/r55955%20%288504%29/results.html
Comment 9 Kenneth Rohde Christiansen 2010-03-13 10:23:11 PST
I cq'ed 2 other patches providing fixed to AC. When these land, could you retest with this patch?

It would be nice to have this enabled by default so that we can actually make sure that our AC works as expected.
Comment 10 Csaba Osztrogonác 2010-03-13 15:48:23 PST
(In reply to comment #9)
> I cq'ed 2 other patches providing fixed to AC. When these land, could you
> retest with this patch?
I retested r55970 with this patch, but unfortunately the three animation test still fail.
Comment 11 Jesus Sanchez-Palencia 2010-03-16 14:13:22 PDT
Is this Mac OS specific?
If not, could you please change the OS so we don't add another item to the Mac specific bugs list?

(I can't edit the bugzilla entry, otherwise I would edit this.)
Comment 12 Csaba Osztrogonác 2010-03-16 14:18:59 PDT
(In reply to comment #11)
> Is this Mac OS specific?
No, I changed it.
Comment 13 Csaba Osztrogonác 2010-03-16 14:43:02 PDT
Created attachment 50840 [details]
enable AC, skip failing tests

Kenneth told me on #qtwebkit that enabling AC is 
more important than these three failing tests now,
so we can enable AC, and skip them until fix.

Thank for Noam to file another bug for fixing these tests.
Comment 14 Csaba Osztrogonác 2010-03-17 05:48:48 PDT
Comment on attachment 50840 [details]
enable AC, skip failing tests

I landed it manually, because CQ didin't work for a day:
http://trac.webkit.org/changeset/56106
Comment 15 Jesus Sanchez-Palencia 2010-03-17 13:06:59 PDT
Follow up patch for QtLauncher is here:
https://bugs.webkit.org/show_bug.cgi?id=36234
Comment 16 Simon Hausmann 2010-03-18 15:51:32 PDT
*** Bug 35522 has been marked as a duplicate of this bug. ***
Comment 17 Kenneth Rohde Christiansen 2010-03-24 12:10:26 PDT
Can this bug be closed? or what is missing?
Comment 18 Csaba Osztrogonác 2010-03-24 12:27:38 PDT
(In reply to comment #17)
> Can this bug be closed? or what is missing?

Fixing the 3 tests was broken by enabling AC is missing:
https://bugs.webkit.org/show_bug.cgi?id=36191

From now this bug depends on b36191, I suggest we shouldn't close it until fix.
Comment 19 Kenneth Rohde Christiansen 2010-04-06 12:18:27 PDT
Can we close this one? or what is missing?
Comment 20 Csaba Osztrogonác 2010-05-14 06:06:59 PDT
(In reply to comment #19)
> Can we close this one? or what is missing?
Yes, because all test work and are unskipped now.
Comment 21 Simon Hausmann 2010-05-14 08:42:22 PDT
Revision r55955 cherry-picked into qtwebkit-2.0 with commit 75fb772135fa93687ef31e76224d0d5b3c0bbd5f