Bug 109540 - [TexMap] Apply frames-per-second debug counter to WK1.
Summary: [TexMap] Apply frames-per-second debug counter to WK1.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dongseong Hwang
URL:
Keywords:
Depends on: 107942
Blocks: 109428
  Show dependency treegraph
 
Reported: 2013-02-11 19:59 PST by Dongseong Hwang
Modified: 2013-02-12 05:51 PST (History)
6 users (show)

See Also:


Attachments
Patch (28.88 KB, patch)
2013-02-11 20:09 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (28.90 KB, patch)
2013-02-11 20:37 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (29.03 KB, patch)
2013-02-12 00:11 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Patch (29.09 KB, patch)
2013-02-12 00:24 PST, Dongseong Hwang
no flags Details | Formatted Diff | Diff
Complementary patch (basysKom copyright) (2.20 KB, patch)
2013-02-12 04:47 PST, Bruno Abinader (history only)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dongseong Hwang 2013-02-11 19:59:44 PST
r142524 implemented frames-per-second debug counter on WK2. This patch applies frames-per-second debug counter to WK1 also.
Comment 1 Dongseong Hwang 2013-02-11 20:09:51 PST
Created attachment 187762 [details]
Patch
Comment 2 Dongseong Hwang 2013-02-11 20:37:29 PST
Created attachment 187766 [details]
Patch
Comment 3 Noam Rosenthal 2013-02-11 23:13:05 PST
Comment on attachment 187766 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187766&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:48
> +void TextureMapperFPSCounter::update(TextureMapper* textureMapper, const FloatPoint& location, const TransformationMatrix& matrix)

The name update is a bit ambiguous.
How about incrementFrameCountAndDisplay
Comment 4 Noam Rosenthal 2013-02-11 23:32:48 PST
Comment on attachment 187766 [details]
Patch

I'm not sure that painting to clip.location() is always right. 
In WK2 this is somewhat acceptable because the viewport is always painted in full, but in WK1 it would cause strange artifacts if partial updates occur.
Comment 5 Dongseong Hwang 2013-02-12 00:02:58 PST
(In reply to comment #3)
> (From update of attachment 187766 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187766&action=review
> The name update is a bit ambiguous.
> How about incrementFrameCountAndDisplay

ok, I'll change incrementFrameCountAndDisplay

(In reply to comment #4)
> (From update of attachment 187766 [details])
> I'm not sure that painting to clip.location() is always right. 
> In WK2 this is somewhat acceptable because the viewport is always painted in full, but in WK1 it would cause strange artifacts if partial updates occur.

In WK1, we will set location to IntPoint::zero().
Comment 6 Dongseong Hwang 2013-02-12 00:11:57 PST
Created attachment 187788 [details]
Patch
Comment 7 Dongseong Hwang 2013-02-12 00:24:18 PST
Created attachment 187791 [details]
Patch
Comment 8 Dongseong Hwang 2013-02-12 00:26:14 PST
Comment on attachment 187791 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187791&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:48
> +void TextureMapperFPSCounter::updateFPSAndDisplay(TextureMapper* textureMapper, const FloatPoint& location, const TransformationMatrix& matrix)

Rename from incrementFrameCountAndDisplay to updateFPSAndDisplay because incrementing frame count is implementation detail and the goal is to update FPS.
Comment 9 Noam Rosenthal 2013-02-12 00:46:52 PST
Comment on attachment 187791 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=187791&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:4
> +    Copyright (C) 2012, 2013 Company 100, Inc.
> +

Bruno from BasysKom has written the original change.
Bruno, if you want to add a BasysKom copyright header feel free :)
Comment 10 Dongseong Hwang 2013-02-12 00:54:15 PST
(In reply to comment #9)
> (From update of attachment 187791 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=187791&action=review
> 
> > Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:4
> > +    Copyright (C) 2012, 2013 Company 100, Inc.
> > +
> 
> Bruno from BasysKom has written the original change.
> Bruno, if you want to add a BasysKom copyright header feel free :)

Aha. I'm waiting for Bruno's comment before cq+ :)

If you comment, I'll cq+ with the copyright!
Comment 11 Noam Rosenthal 2013-02-12 00:55:14 PST
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 187791 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=187791&action=review
> > 
> > > Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:4
> > > +    Copyright (C) 2012, 2013 Company 100, Inc.
> > > +
> > 
> > Bruno from BasysKom has written the original change.
> > Bruno, if you want to add a BasysKom copyright header feel free :)
> 
> Aha. I'm waiting for Bruno's comment before cq+ :)
> 
> If you comment, I'll cq+ with the copyright!
I think you can commit. Bruno can add the copyright later if he wishes.
Comment 12 Dongseong Hwang 2013-02-12 01:06:02 PST
(In reply to comment #11)
> I think you can commit. Bruno can add the copyright later if he wishes.

ok, could you cq+? I don't have a committer authority yet.
Comment 13 WebKit Review Bot 2013-02-12 01:36:19 PST
Comment on attachment 187791 [details]
Patch

Clearing flags on attachment: 187791

Committed r142595: <http://trac.webkit.org/changeset/142595>
Comment 14 WebKit Review Bot 2013-02-12 01:36:24 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 Bruno Abinader (history only) 2013-02-12 04:31:13 PST
(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (From update of attachment 187791 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=187791&action=review
> > > 
> > > > Source/WebCore/platform/graphics/texmap/TextureMapperFPSCounter.cpp:4
> > > > +    Copyright (C) 2012, 2013 Company 100, Inc.
> > > > +
> > > 
> > > Bruno from BasysKom has written the original change.
> > > Bruno, if you want to add a BasysKom copyright header feel free :)
> > 
> > Aha. I'm waiting for Bruno's comment before cq+ :)
> > 
> > If you comment, I'll cq+ with the copyright!
> I think you can commit. Bruno can add the copyright later if he wishes.

Hi guys, sorry, different timezones here! (Manaus, Brazil -04:00 GMT). Yes, please do, shall I add a one-liner patch to this bug to add the copyright info?
Comment 16 Bruno Abinader (history only) 2013-02-12 04:46:57 PST
Reopening to attach new patch.
Comment 17 Bruno Abinader (history only) 2013-02-12 04:47:04 PST
Created attachment 187832 [details]
Complementary patch (basysKom copyright)
Comment 18 WebKit Review Bot 2013-02-12 05:50:59 PST
Comment on attachment 187832 [details]
Complementary patch (basysKom copyright)

Clearing flags on attachment: 187832

Committed r142608: <http://trac.webkit.org/changeset/142608>
Comment 19 WebKit Review Bot 2013-02-12 05:51:04 PST
All reviewed patches have been landed.  Closing bug.