Closed Bug 1444430 Opened 6 years ago Closed 6 years ago

Add compositor-agnostic infrastructure to submit screenshots as profiler markers

Categories

(Core :: Graphics: Layers, enhancement)

Product:
Core
Component:
Graphics: Layers
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox60 --- wontfix
firefox61 --- fixed

People

(Reporter: mstange, Assigned: mstange)

References

Details

Attachments

(4 files)

Bug 1444430 - Remove a copy in EncodeSourceSurfaceInternal.
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
Bug 1444430 - Extend the API of gfxUtils::EncodeSourceSurface to support encoding into a string.
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details
Bug 1444430 - Add a profiler feature called "screenshots".
59 bytes, text/x-review-board-request
n.nethercote
: review+
Details
Bug 1444430 - Add a ProfilerScreenshots class which can be used to submit DataSourceSurfaces to the profiler.
59 bytes, text/x-review-board-request
jrmuizel
: review+
Details 
      No description provided.
Blocks: 1444432
Blocks: 1444434
Blocks: 1444447
Blocks: 1444449
Blocks: 1444452
Comment on attachment 8962587 [details]
Bug 1444430 - Remove a copy in EncodeSourceSurfaceInternal.

https://reviewboard.mozilla.org/r/231402/#review237308

::: gfx/thebes/gfxUtils.cpp:947
(Diff revision 1)
>  EncodeSourceSurfaceInternal(SourceSurface* aSurface,
>                             const nsACString& aMimeType,
>                             const nsAString& aOutputOptions,
>                             gfxUtils::BinaryOrData aBinaryOrData,
>                             FILE* aFile,
> -                           nsCString* aStrOut)
> +                           nsACString* aStrOut)

This function should really be split up.
Attachment #8962587 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8962589 [details]
Bug 1444430 - Add a profiler feature called "screenshots".

https://reviewboard.mozilla.org/r/231406/#review237344

::: browser/components/extensions/schemas/geckoProfiler.json:37
(Diff revision 2)
>            "restyle",
>            "stackwalk",
>            "tasktracer",
>            "threads",
> -          "trackopts"
> +          "trackopts",
> +          "screenshots"

Please keep this list in alphabetical order.

::: tools/profiler/public/GeckoProfiler.h:137
(Diff revision 2)
>    \
>    /* Have the JavaScript engine track JIT optimizations. */ \
> -  macro(10, "trackopts", TrackOptimizations)
> +  macro(10, "trackopts", TrackOptimizations) \
> +  \
> +  /* Take a snapshot of the window on every composition. */ \
> +  macro(11, "screenshots", Screenshots)

Ditto.
Attachment #8962589 - Flags: review?(n.nethercote) → review+
Comment on attachment 8962588 [details]
Bug 1444430 - Extend the API of gfxUtils::EncodeSourceSurface to support encoding into a string.

https://reviewboard.mozilla.org/r/231404/#review237990

::: gfx/thebes/gfxUtils.cpp:1111
(Diff revision 3)
>                                BinaryOrData aBinaryOrData,
> -                              FILE* aFile)
> +                              FILE* aFile,
> +                              nsACString* aString)
>  {
>    return EncodeSourceSurfaceInternal(aSurface, aMimeType, aOutputOptions,
> -                                     aBinaryOrData, aFile, nullptr);
> +                                     aBinaryOrData, aFile, aString);

Let's just rename Internal instead.
Attachment #8962588 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8962590 [details]
Bug 1444430 - Add a ProfilerScreenshots class which can be used to submit DataSourceSurfaces to the profiler.

https://reviewboard.mozilla.org/r/231408/#review238006

::: gfx/layers/ProfilerScreenshots.cpp:63
(Diff revision 3)
> +
> +  aPopulateSurface(backingSurface);
> +
> +  if (!mThread) {
> +    nsresult rv =
> +      NS_NewNamedThread("ProfScreenshot", getter_AddRefs(mThread));

Please add a common do 'eventually' thread.
Attachment #8962590 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8962590 [details]
Bug 1444430 - Add a ProfilerScreenshots class which can be used to submit DataSourceSurfaces to the profiler.

https://reviewboard.mozilla.org/r/231408/#review238006

> Please add a common do 'eventually' thread.

I filed bug 1450059 about this.
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/4e54db9e52b5
Remove a copy in EncodeSourceSurfaceInternal. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/643af79826b8
Extend the API of gfxUtils::EncodeSourceSurface to support encoding into a string. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/a08a2cbad087
Add a profiler feature called "screenshots". r=njn
https://hg.mozilla.org/integration/autoland/rev/8b6f8d93e9dd
Add a ProfilerScreenshots class which can be used to submit DataSourceSurfaces to the profiler. r=jrmuizel
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: