662649 - Fixup fps display

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Fix up fps code.

This fixes a couple of problems:

1. switches to a 32 bit type
2. adds a bit of documentation
3. frees the temporary buffer

Comment 1

[Joe Drew (not getting mail)]

Comment on attachment 537881 [details] [diff] [review]
Fix up fps code.

Review of attachment 537881 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/opengl/LayerManagerOGL.cpp
@@ +575,5 @@
>        0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,   0,
>      };
>  
> +    // convert from 8 bit to 32 bit so that we don't have to write the text out in 32 bit format
> +    // we rely on int being 32 bits */

please remove trailing */

Comment 2

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/mozilla-central/rev/3b95f57892ba and children

Comment 3

[Nicholas Nethercote [inactive]]

> Fix up fps code.
> 
> This fixes a couple of problems:
> 
> 1. switches to a 32 bit type
> 2. adds a bit of documentation
> 3. frees the temporary buffer

What is "fps"?  First-person shooter?

I assume the temporary buffer was being leaked?  How big is it, how often was it being leaked?

Comment 4

[Joe Drew (not getting mail)]

"fps" is "Frames per second." This code lets us monitor how quickly we're drawing to the screen. It's not on by default, and, unless I am misremembering, before Jeff's changesets, it wasn't even possible to turn it on.

Did you have any reason to reopen this?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

We only leaked when a pref was set.

Comment 6

[Nicholas Nethercote [inactive]]

(In reply to comment #4)
> 
> Did you have any reason to reopen this?

It wasn't resolved, so I resolved it, but then I got cold feet because the link in comment 2 didn't seem to include the extra free() call that the attached patch did.  But I guess it went in in one of the changes not listed?

Comment 7

[AndreiD[QA]]

I was trying to see if this is fixed for Fx7 since the flag "status-firefox7" is set to "fixed", but I couldn't.
Is there a test case or any steps / guidelines for this bug that can be used to verify the fix? Thanks

Comment 8

[u279076]

qa-: no QA fix verification needed