Bug 40381 - [Qt] for debugging purposes I'm contributing back my FPS counter in the AnimationQtBase
Summary: [Qt] for debugging purposes I'm contributing back my FPS counter in the Anima...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks: 38744
  Show dependency treegraph
 
Reported: 2010-06-09 11:49 PDT by Sam Magnuson
Modified: 2010-07-08 23:45 PDT (History)
4 users (show)

See Also:


Attachments
Simply start a timer and count frames in the AnimationQtBase and spit out the FPS count at the end of a single animation. (4.15 KB, patch)
2010-06-09 12:39 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Proposed patch with proper coding style (5.10 KB, patch)
2010-06-12 10:24 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Pull from origin and reapply patch, fix style issues. (5.04 KB, patch)
2010-06-16 09:28 PDT, Sam Magnuson
kenneth: review-
Details | Formatted Diff | Diff
Fix comments in last review (5.03 KB, patch)
2010-06-16 10:31 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
rebase and rediff (5.03 KB, patch)
2010-06-21 15:25 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Rediff against trunk (4.22 KB, patch)
2010-06-22 16:11 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff
Patch (3.84 KB, patch)
2010-06-25 11:24 PDT, Sam Magnuson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Magnuson 2010-06-09 11:49:55 PDT
Just hoping to have some way to compare changes in the graphicslayer between two sets of changes
Comment 1 Sam Magnuson 2010-06-09 12:39:01 PDT
Created attachment 58277 [details]
 Simply start a timer and count frames in the AnimationQtBase and spit out the FPS count at the end of a single animation.
Comment 2 Kenneth Rohde Christiansen 2010-06-11 18:52:06 PDT
No'am, is this useful?
Comment 3 Noam Rosenthal 2010-06-11 19:27:10 PDT
It's not necessarily useful for me; but if it's useful for Sam and it's cleaned up for style a bit I have no objection.
Comment 4 Sam Magnuson 2010-06-12 10:24:06 PDT
Created attachment 58559 [details]
Proposed patch with proper coding style
Comment 5 Jędrzej Nowacki 2010-06-16 06:18:56 PDT
Comment on attachment 58559 [details]
Proposed patch with proper coding style

Sam, the patch cann't be applied (you can check why by clicking on the violet "style" button)

there are a few coding style issues, could you fix them? What about autotests?
Comment 6 Sam Magnuson 2010-06-16 09:28:45 PDT
Created attachment 58898 [details]
Pull from origin and reapply patch, fix style issues.
Comment 7 Sam Magnuson 2010-06-16 09:29:46 PDT
(In reply to comment #5)
> (From update of attachment 58559 [details])
> Sam, the patch cann't be applied (you can check why by clicking on the violet "style" button)
> 

Thanks for that, I've pulled and resubmitted the patch.

> there are a few coding style issues, could you fix them? 

Fixed.

> What about autotests?

I noted in the ChangeLog now that this is only a debugging aid and doesn't need a test.
Comment 8 Kenneth Rohde Christiansen 2010-06-16 09:57:18 PDT
Comment on attachment 58898 [details]
Pull from origin and reapply patch, fix style issues.

In the future please make style changes not belonging to the patch itself in a separate patch

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:45
 +  #define QT_DEBUG_FPS 1
You really want to summit it with 1 here?


WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1131
 +          if ( newState == Running && oldState == Stopped ) {
wrong style

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1135
 +          } else if ( newState == Stopped && oldState == Running ) {
here as well

WebCore/platform/graphics/qt/GraphicsLayerQt.cpp:1192
 +      int   m_frames;
here as well
Comment 9 Noam Rosenthal 2010-06-16 09:59:02 PDT
Agree with kenneth, this should be an opt-in (disabled by default)
Comment 10 Sam Magnuson 2010-06-16 10:31:55 PDT
Created attachment 58904 [details]
Fix comments in last review
Comment 11 WebKit Commit Bot 2010-06-19 05:15:16 PDT
Comment on attachment 58904 [details]
Fix comments in last review

Rejecting patch 58904 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christiansen', u'--force']" exit_code: 1
Last 500 characters of output:
hangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp
Hunk #3 FAILED at 335.
Hunk #4 FAILED at 658.
Hunk #5 FAILED at 932.
Hunk #6 succeeded at 1244 with fuzz 1 (offset 119 lines).
Hunk #7 succeeded at 1299 with fuzz 2 (offset 118 lines).
Hunk #8 succeeded at 1360 with fuzz 1 (offset 120 lines).
Hunk #9 succeeded at 1411 (offset 121 lines).
3 out of 9 hunks FAILED -- saving rejects to file WebCore/platform/graphics/qt/GraphicsLayerQt.cpp.rej

Full output: http://webkit-commit-queue.appspot.com/results/3286377
Comment 12 Sam Magnuson 2010-06-21 15:25:47 PDT
Created attachment 59300 [details]
rebase and rediff
Comment 13 Sam Magnuson 2010-06-21 15:26:27 PDT
(In reply to comment #11)
> (From update of attachment 58904 [details])
> Rejecting patch 58904 from commit-queue.
> 

I'm not clear why this went wrong at all - I didn't see any conflicts in the change. I've rebase'd to origin and done the diff again, hope that solves it.
Comment 14 Noam Rosenthal 2010-06-21 16:18:14 PDT
Comment on attachment 59300 [details]
rebase and rediff

Clearing the flags, it still doesn't apply cleanly, we're working on it :)
Comment 15 Eric Seidel (no email) 2010-06-22 13:23:54 PDT
Comment on attachment 58904 [details]
Fix comments in last review

Cleared Kenneth Rohde Christiansen's review+ from obsolete attachment 58904 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 16 Sam Magnuson 2010-06-22 16:11:17 PDT
Created attachment 59435 [details]
Rediff against trunk
Comment 17 WebKit Commit Bot 2010-06-25 10:13:31 PDT
Comment on attachment 59435 [details]
Rediff against trunk

Rejecting patch 59435 from commit-queue.

Unexpected failure when processing patch!  Please file a bug against webkit-patch.
Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', 'land-attachment', '--force-clean', '--build', '--non-interactive', '--ignore-builders', '--build-style=both', '--quiet', 59435, '--test', '--parent-command=commit-queue', '--no-update']" exit_code: 1
Last 500 characters of output:
webkitpy/tool/commands/stepsequence.py", line 60, in _run
    step(tool, options).run(state)
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 68, in run
    if self._has_valid_reviewer(changelog_entry):
  File "/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/webkitpy/tool/steps/validatereviewer.py", line 50, in _has_valid_reviewer
    if changelog_entry.reviewer():
AttributeError: 'NoneType' object has no attribute 'reviewer'
Comment 18 Adam Barth 2010-06-25 11:14:27 PDT
Comment on attachment 59435 [details]
Rediff against trunk

+        2010-06-09  Sam Magnuson  <smagnuson@netflix.com>

Your ChangeLog is malformed.
Comment 19 Sam Magnuson 2010-06-25 11:24:34 PDT
Created attachment 59781 [details]
Patch
Comment 20 Sam Magnuson 2010-06-25 11:24:51 PDT
(In reply to comment #18)
> (From update of attachment 59435 [details])
> +        2010-06-09  Sam Magnuson  <smagnuson@netflix.com>
> 
> Your ChangeLog is malformed.

Fixed.
Comment 21 WebKit Commit Bot 2010-07-08 23:45:47 PDT
Comment on attachment 59781 [details]
Patch

Clearing flags on attachment: 59781

Committed r62899: <http://trac.webkit.org/changeset/62899>
Comment 22 WebKit Commit Bot 2010-07-08 23:45:53 PDT
All reviewed patches have been landed.  Closing bug.