Yay refactoring! I wanted to have FPS data, layer borders, and I wanted to add frame colorbars. I didn't want to do them per-platform. Thus, I called in REFACTOR-MAN to helped me out! (Worst superhero ever.)

This refactors the diagnostics into Compositor, and adds color bar display. Diagnostics only work for new-world OMTC compositors, so all the more reason to get there.

This moves the FPS display into generic code. Currently only fully implemented for D3D11, it needs DataTextureSourceOGL to be done which should be trivial. Will do it shortly.

Comment on attachment 752599 [details] [diff] [review] Part 2, move FPS display into generic code Review of attachment 752599 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/Factory.cpp @@ +492,5 @@ > + > + RefPtr<SourceSurfaceRawData> newSurf = new SourceSurfaceRawData(); > + > + if (newSurf->InitWrappingData(data, aSize, stride, aFormat, true)) { > + return newSurf; should be newSurf.forget(); ::: gfx/layers/FPPStats.cpp @@ +12,5 @@ > + > +gfx::SourceSurfaceRawData* > +FPSUtils::AllocateFPSSurface() > +{ > +} file should have gone away

Comment on attachment 752599 [details] [diff] [review] Part 2, move FPS display into generic code Review of attachment 752599 [details] [diff] [review]: ----------------------------------------------------------------- waiting for an updated version of this patch

Comment on attachment 752599 [details] [diff] [review] Part 2, move FPS display into generic code Review of attachment 752599 [details] [diff] [review]: ----------------------------------------------------------------- waiting for an updated version of this patch ::: gfx/layers/composite/TextureHost.h @@ +116,5 @@ > + /* Upload a new DataSourceSurface to this DataTextureSource. It > + * must have the same dimensions and format as what it was > + * constructed with. > + */ > + virtual bool UploadDataSourceSurface(gfx::DataSourceSurface *aNewSurface) = 0; Actually, I'd like that this method take alsoe these two arguments: - nsIntRegion* aDestRegion = nullptr // nullptr means the entire thing - gfx::IntPoint* aSrcOffset = nullptr // nullptr means {0,0} offset This is because for another bug we need this DataTextureSource abstraction with these two parameters to do partial updates.

Hi Vlad, In many case, user cares not only AVG FPS, but also STEDV of FPS. We may add more on screen sprite to reveal benchmark data for developer. Please also considerate this possibility of doing this.

To draw FPS in HwcCompositor, it's helpful if the hoster of HwcCompositor & CompositorOGL, which is LayerManagerOGL, is in charge of reading preference value and enable/ disable FPS drawing for both HwcCompositor and CompositorOGL.

(In reply to C.J. Ku[:CJKu] from comment #8) > To draw FPS in HwcCompositor, it's helpful if the hoster of HwcCompositor & > CompositorOGL, which is LayerManagerOGL, is in charge of reading preference > value and enable/ disable FPS drawing for both HwcCompositor and > CompositorOGL. We can only do that on B2G, though. for other platforms only the main thread is allowed to directly access prefs.

Reviving this, now that compositor is much more stable. This just adds drawing color bars based on the current frame number. In the future, we can maybe use the actual transaction number from bug 854421 to know exactly when something made it to the screen (though that won't be enough, not all frames have a transaction).

Comment on attachment 8416102 [details] [diff] [review] color-bars.patch Review of attachment 8416102 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, sorry for the delay.