Add FPS counter like fraps

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: romaxa)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Description

7 years ago
If you run firefox with fraps running it adds a fps counter to all of the windows it would be handy if we had this functionality built in.
(Reporter)

Comment 1

7 years ago
Created attachment 492758 [details] [diff] [review]
An OpenGL version of this
(Reporter)

Comment 2

7 years ago
Created attachment 495092 [details] [diff] [review]
v2
Attachment #492758 - Attachment is obsolete: true
(Reporter)

Comment 3

7 years ago
Comment on attachment 495092 [details] [diff] [review]
v2

I'll put:

+
+  mFPS.DrawFPS(mGLContext, GetCopy2DProgram());
+

under an #if 0
Attachment #495092 - Flags: review?(joe)
Comment on attachment 495092 [details] [diff] [review]
v2

>diff --git a/gfx/layers/opengl/LayerManagerOGL.cpp b/gfx/layers/opengl/LayerManagerOGL.cpp

>+void LayerManagerOGL::FPSState::DrawFPS(
>+    GLContext* context,
>+    CopyProgram *copyprog
>+    ){

Can you put the arguments list on a single line, and the { on a newline, please?

>+  static TimeStamp last;
>+  static int fcount = 0;
>+  static int fps;
>+  static bool initialized;

Shouldn't you be using the FPSState member variables?

>diff --git a/gfx/layers/opengl/LayerManagerOGL.h b/gfx/layers/opengl/LayerManagerOGL.h

>+  struct FPSState
>+  {
>+      static GLuint texture;
>+      int fps;
>+      bool initialized;

Either both initialized and texture should be static, or neither should be.
Attachment #495092 - Flags: review?(joe) → review-
(Assignee)

Comment 5

6 years ago
Created attachment 530303 [details] [diff] [review]
v3. restyle, + added preference

Fixed last comments, common restyle, also added preference, so we can test/check any time by just setting pref and restarting fennec
Attachment #495092 - Attachment is obsolete: true
Attachment #530303 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #530303 - Flags: review?(joe)
Attachment #530303 - Flags: review?(jmuizelaar)
Attachment #530303 - Flags: review?
Comment on attachment 530303 [details] [diff] [review]
v3. restyle, + added preference

Review of attachment 530303 [details] [diff] [review]:

::: gfx/layers/opengl/LayerManagerOGL.cpp
@@ +80,5 @@
   , mBackBufferFBO(0)
   , mBackBufferTexture(0)
   , mBackBufferSize(-1, -1)
   , mHasBGRA(0)
+  , mFPS()

You don't need this, since the default constructor's called anyways - right?

@@ +593,5 @@
+  const Vertex2D vertices[] = {
+    { -1.0, 1.0 - 42. / viewport[3]},
+    { -1.0, 1.0},
+    { -1.0 + 22. / viewport[2], 1.0 - 42. / viewport[3]},
+    { -1.0 + 22. / viewport[2], 1.0},

Can you add spaces before the ending "}," on all these vertices?

@@ +653,5 @@
+
+  context->fEnableVertexAttribArray(vcattr);
+  context->fEnableVertexAttribArray(tcattr);
+
+

extra blank line here.

@@ +667,5 @@
+
+  context->fDrawArrays(LOCAL_GL_TRIANGLE_STRIP, 0, 12);
+}
+
+

extra blank line here too.
Attachment #530303 - Flags: review?(joe) → review+
(Assignee)

Comment 7

6 years ago
Created attachment 530444 [details] [diff] [review]
Fixed nits, version to push
(Assignee)

Updated

6 years ago
Attachment #530303 - Flags: review?(jmuizelaar)
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Attachment #530444 - Flags: review+
Attachment #530303 - Attachment is obsolete: true

Updated

6 years ago
Assignee: nobody → romaxa
Keywords: checkin-needed
Whiteboard: [fixed in cedar]
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Backed out from cedar because of build bustage.
See:
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1304673177.1304674061.19195.gz

Seems to fail only on Linux.
Whiteboard: [fixed in cedar]
(Assignee)

Comment 9

6 years ago
Created attachment 530819 [details] [diff] [review]
Fixed compilation on linux, typing problem
Attachment #530819 - Flags: review?(jmuizelaar)
(Assignee)

Comment 10

6 years ago
Created attachment 530820 [details] [diff] [review]
interdiff from previous patch
Attachment #530444 - Attachment is obsolete: true
(Assignee)

Comment 11

6 years ago
I was testing this patch originally only on scratchbox, now it compiles on linux too,
(Reporter)

Comment 12

6 years ago
Comment on attachment 530820 [details] [diff] [review]
interdiff from previous patch

Can you explain the reason for increasing the size of all of theses types?
(Assignee)

Comment 13

6 years ago
> Can you explain the reason for increasing the size of all of theses types?
Because it fails to compile, see:
http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1304673177.1304674061.19195.gz

576:5: error: narrowing conversion of '255' from 'int' to 'char' inside { }
606:3: error: narrowing conversion of '(1.0e+0 - (4.2e+1 / (double)viewport[3]))' from 'double' to 'float' inside { }
629:3: error: narrowing conversion of '(((double)v100 * 4.0e+0) / 6.4e+1)' from 'double' to 'const GLfloat' inside { }
(Assignee)

Comment 14

6 years ago
I don't see this error on scratchbox mobile builds, but see it on Desktop Linux
(Assignee)

Comment 15

6 years ago
Found similar issue discussed here:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44500#c18
(Reporter)

Comment 16

6 years ago
(In reply to comment #13)
> > Can you explain the reason for increasing the size of all of theses types?
> Because it fails to compile, see:
> http://tinderbox.mozilla.org/showlog.cgi?log=Cedar/1304673177.1304674061.
> 19195.gz
> 
> 576:5: error: narrowing conversion of '255' from 'int' to 'char' inside { }
> 606:3: error: narrowing conversion of '(1.0e+0 - (4.2e+1 /
> (double)viewport[3]))' from 'double' to 'float' inside { }
> 629:3: error: narrowing conversion of '(((double)v100 * 4.0e+0) / 6.4e+1)'
> from 'double' to 'const GLfloat' inside { }

You should be able to fix this by using 'unsigned char' for text and switching from double literals to float literals.
(Reporter)

Updated

6 years ago
Attachment #530819 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 17

6 years ago
Created attachment 531600 [details] [diff] [review]
Fixed compilation on linux, typing problem
Attachment #530819 - Attachment is obsolete: true
Attachment #530820 - Attachment is obsolete: true
Attachment #531600 - Flags: review?(jmuizelaar)
(Assignee)

Comment 18

6 years ago
checked on try and now it compiles fine.
(Assignee)

Comment 19

6 years ago
Created attachment 531602 [details] [diff] [review]
interdiff from last r+ patch
(Reporter)

Updated

6 years ago
Attachment #531600 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 20

6 years ago
http://hg.mozilla.org/mozilla-central/rev/7febcf91de21
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
How to see this FPS counter on firefox windows ?
I am usinf the latest hourly , which has this bug landed.
(Assignee)

Comment 22

6 years ago
you should set layers.acceleration.draw-fps=true pref, but implementation is done only for OGL backend, so if you need D3D or something like that,  we should add related implementation for D3D backend

Updated

6 years ago
Blocks: 683709

Updated

6 years ago
Blocks: 683745
Blocks: 780467
(In reply to Oleg Romashin (:romaxa) from comment #22)
> you should set layers.acceleration.draw-fps=true pref, but implementation is
> done only for OGL backend, so if you need D3D or something like that,  we
> should add related implementation for D3D backend

I'm also on Windows. I've tried flipping the preference but nothing happened, then I stumbled on this bug/comment. I've opened bug 780467 for tracking progress on DirectX/Windows.
You need to log in before you can comment on or make changes to this bug.