Last Comment Bug 662649 - Fixup fps display
: Fixup fps display
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on:
Blocks: mlk-fx7
  Show dependency treegraph
 
Reported: 2011-06-07 15:03 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2011-09-22 15:07 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
Fix up fps code. (2.33 KB, patch)
2011-06-07 15:03 PDT, Jeff Muizelaar [:jrmuizel]
joe: review+
Details | Diff | Review

Description Jeff Muizelaar [:jrmuizel] 2011-06-07 15:03:32 PDT
Created attachment 537881 [details] [diff] [review]
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) 2011-06-07 15:56:41 PDT
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] 2011-06-23 09:30:16 PDT
http://hg.mozilla.org/mozilla-central/rev/3b95f57892ba and children
Comment 3 Nicholas Nethercote [:njn] 2011-06-24 01:51:59 PDT
> 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) 2011-06-24 13:23:16 PDT
"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] 2011-06-24 13:28:17 PDT
We only leaked when a pref was set.
Comment 6 Nicholas Nethercote [:njn] 2011-06-24 14:56:17 PDT
(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] 2011-08-25 04:43:35 PDT
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 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-22 15:07:30 PDT
qa-: no QA fix verification needed

Note You need to log in before you can comment on or make changes to this bug.