Bug 109540 – [TexMap] Apply frames-per-second debug counter to WK1.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dongseong Hwang]

r142524 implemented frames-per-second debug counter on WK2. This patch applies frames-per-second debug counter to WK1 also.

[Dongseong Hwang]

Created attachment 187762 [details]
Patch

[Dongseong Hwang]

Created attachment 187766 [details]
Patch

[Noam Rosenthal]

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

[Noam Rosenthal]

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.

[Dongseong Hwang]

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

[Dongseong Hwang]

Created attachment 187788 [details]
Patch

[Dongseong Hwang]

Created attachment 187791 [details]
Patch

[Dongseong Hwang]

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.

[Noam Rosenthal]

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 :)

[Dongseong Hwang]

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

[Noam Rosenthal]

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

[Dongseong Hwang]

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

[WebKit Review Bot]

Comment on attachment 187791 [details]
Patch

Clearing flags on attachment: 187791

Committed r142595: <http://trac.webkit.org/changeset/142595>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Bruno Abinader (history only)]

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

[Bruno Abinader (history only)]

Reopening to attach new patch.

[Bruno Abinader (history only)]

Created attachment 187832 [details]
Complementary patch (basysKom copyright)

[WebKit Review Bot]

Comment on attachment 187832 [details]
Complementary patch (basysKom copyright)

Clearing flags on attachment: 187832

Committed r142608: <http://trac.webkit.org/changeset/142608>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.