Closed Bug 874781 Opened 7 years ago Closed 6 years ago

add color bars to layer diagnostics, for vsync checking

Categories

(Core :: Graphics: Layers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: vlad, Assigned: vlad)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file, 2 obsolete files)

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.
Attachment #752598 - Flags: review?(nical.bugzilla)
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.
Attachment #752599 - Flags: review?(nical.bugzilla)
Attachment #752598 - Flags: review?(nical.bugzilla) → review+
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
Attachment #752599 - Flags: review?(nical.bugzilla)
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.
Blocks: 909204
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.
See Also: → 963821
Summary: refactor layer diagnostics, unify in Compositor → add color bars to layer diagnostics, for vsync checking
Attached patch color-bars.patchSplinter Review
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).
Attachment #752598 - Attachment is obsolete: true
Attachment #752599 - Attachment is obsolete: true
Attachment #8416102 - Flags: review?(bgirard)
Comment on attachment 8416102 [details] [diff] [review]
color-bars.patch

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

Looks great, sorry for the delay.
Attachment #8416102 - Flags: review?(bgirard) → review+
See Also: → 947314
https://hg.mozilla.org/mozilla-central/rev/1073160bd1e4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.