1052346 - layers.acceleration.draw-fps reports bogus values.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jukka Jylänki]

Attachment: Capture.PNG

In FF34 Nightly on Windows, 

1. Enable the layers.acceleration.draw-fps pref to enable the FPS counter.
2. Visit https://dl.dropboxusercontent.com/u/40949268/emcc/10kCubes_benchmark/10kCubes.html

3. Check that the rendered page reports "FPS: 60" (this should be true unless you have an extremely weak CPU/GPU)
4. Observe that the FPS counter provided by layers.acceleration.draw-fps (the first value of the three numbers) reports a very low figure, in my tests it fluctuates around 25fps.

Expected: The FPS counter should show the number 60.

I am suspecting that this is just a bug with the FPS counter and that the page itself is actually displaying frames at 60fps. The other possibility is that the page is running its requestAnimationFrame+WebGL logic at 60fps, but only 25 frames get displayed on screen per second and the browser is discarding/not displaying the rest(?)

Comment 1

[Jeff Muizelaar [:jrmuizel]]

The fps code is hitting a bug in the Windows implementation of ToSecondsSigDigits causing it to display the wrong values.

Comment 3

[Jukka Jylänki]

Great find!

Comment 4

[Jukka Jylänki]

This still occurs in today's nightly, reopening as new.

Comment 5

[Jukka Jylänki]

As a slight nit, I am consistently seeing a practice on bugzilla where bugs that are user stories (like this one) get closed as a duplicate of a bug that has a technical non-user-understandable title that works to patch it (in this case, https://bugzilla.mozilla.org/show_bug.cgi?id=1054021 ). This happens to my reports often enough that I'm starting to find that troublesome for me. I think that is a wrong mode of operation and we should never close user story bugs until they are confirmed to be fixed. It is ok to mark this bug to depend on 1054021, but keep it open only close this once it's verified to be fixed.

There are multiple things that have gone wrong here:
 - The patch 1054021 never landed, but went stale without progress. I was unaware that it had been abandoned, and was waiting for that to land.
 - Instead there was another patch https://bugzilla.mozilla.org/show_bug.cgi?id=1054474 that was supposed to fix this. Neither this bug or bug 1054474 was informed about that another patch.
 - Bug 1054474 has already landed, but it does not fix this problem. The problem still exists as reported in the original STR above.

All of the above mistakes are ok and understandable by itself, that's just standard software development, not a big deal. But I want to point out that if we would change the mode of operation to never close original user story bugs immediately as RESOLVED DUPLICATE when there is a (not-yet-merged) patch bug in the works for that, then automatically none of those accidents would have happened. We should never close a user story with STR, until that STR has been verified to be fixed. We should also never close a user story with STR even if it was a duplicate of another report (with same underlying reason), as long as it has different STR.

A user story with an STR should ever be closed once the fix is in distribution and the STR has been successfully verified as working.

Comment 6

[Kelsey Gilbert [:jgilbert]]

I believe I agree. I support using the 'depends on' field unless the bug is clearly the same in scope.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

(In reply to Jukka Jylänki from comment #5)
> There are multiple things that have gone wrong here:
>  - The patch 1054021 never landed, but went stale without progress. I was
> unaware that it had been abandoned, and was waiting for that to land.

The patch was waiting for review not abandoned. But I agree, my dupping of the bug was over zealous.


> A user story with an STR should ever be closed once the fix is in
> distribution and the STR has been successfully verified as working.

That's fine as long as the reporters actually close the user story bugs. As long as that happens, I agree it's better to just use dependencies.

Hopefully the solution in bug 1059803 works for you.

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Jukka, is this working now, or still broken?  The bug it depends on is marked fixed, but I was not sure.

Comment 9

[Jukka Jylänki]

This is not working because the fps meter rendering is broken on Windows. That is bug 1150552.

Comment 10

[Jukka Jylänki]

Verified as fixed now.