Bug 40381 – [Qt] for debugging purposes I'm contributing back my FPS counter in the AnimationQtBase

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Qt

[Sam Magnuson]

Just hoping to have some way to compare changes in the graphicslayer between two sets of changes

[Sam Magnuson]

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.

[Kenneth Rohde Christiansen]

No'am, is this useful?

[Noam Rosenthal]

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.

[Sam Magnuson]

Created attachment 58559 [details]
Proposed patch with proper coding style

[Jędrzej Nowacki]

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?

[Sam Magnuson]

Created attachment 58898 [details]
Pull from origin and reapply patch, fix style issues.

[Sam Magnuson]

(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.

[Kenneth Rohde Christiansen]

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

[Noam Rosenthal]

Agree with kenneth, this should be an opt-in (disabled by default)

[Sam Magnuson]

Created attachment 58904 [details]
Fix comments in last review

[WebKit Commit Bot]

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

[Sam Magnuson]

Created attachment 59300 [details]
rebase and rediff

[Sam Magnuson]

(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.

[Noam Rosenthal]

Comment on attachment 59300 [details]
rebase and rediff

Clearing the flags, it still doesn't apply cleanly, we're working on it :)

[Eric Seidel (no email)]

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.

[Sam Magnuson]

Created attachment 59435 [details]
Rediff against trunk

[WebKit Commit Bot]

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'

[Adam Barth]

Comment on attachment 59435 [details]
Rediff against trunk

+        2010-06-09  Sam Magnuson  <[e-mail]>

Your ChangeLog is malformed.

[Sam Magnuson]

Created attachment 59781 [details]
Patch

[Sam Magnuson]

(In reply to comment #18)
> (From update of attachment 59435 [details])
> +        2010-06-09  Sam Magnuson  <[e-mail]>
> 
> Your ChangeLog is malformed.

Fixed.

[WebKit Commit Bot]

Comment on attachment 59781 [details]
Patch

Clearing flags on attachment: 59781

Committed r62899: <http://trac.webkit.org/changeset/62899>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.