Closed Bug 959118 Opened 6 years ago Closed 5 years ago

[LayerScope] Combine layer dump and layer scope

Categories

(Core :: Graphics, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: u459114, Assigned: boris)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 11 obsolete files)

1.08 MB, image/png
Details
179.28 KB, patch
boris
: review+
Details | Diff | Splinter Review
We have two separate tools, layer dump and layer scope
Layer dump is to dump the tree architecture and layer information, image buffer is excluded.
Layer scope is to dump layer image.

Combine these two tools into one is useful for gecko and gaia developers.
Assignee: mtseng → boris.chiou
Status: NEW → ASSIGNED
Depends on: 1035045
We also want to dump layer tree on the viewer, so we
can check the layer tree and layerscope together
in the viewer. This can help us resolve more gfx bugs.

In this patch, I only add a part of the layer data to
the protocol buffer packet, and you can check the
.proto file for more information if you want to add
more layer data.

Now, we can display the layer tree and
the following layer information:
1. Common layer data:
  a. layer reference
  b. layer type
  c. shadow clip, transform, and region
  d. clip
  e. transform
  f. visible region
  g. opacity value
  h. content opaque
  i. component content alpha
  j. scrolling bar id
  k. mask layer reference
2. Speficie layer data:
  a. valid region
  b. color
  c. filter type
  d. ref layer id
  e. readback layer size

Try result(only build):
https://tbpl.mozilla.org/?tree=Try&rev=fedb3fd11a18
Attachment #8460080 - Attachment is obsolete: true
The demo of this new viewer.
https://github.com/BorisChiou/layerscope/tree/layerdump
Attachment #8445010 - Attachment is obsolete: true
We also want to dump layer tree on the viewer, so we
can check the layer tree and layerscope together
in the viewer. This can help us resolve more gfx bugs.

In this patch, I only add a part of the layer data to
the protocol buffer packet, and you can check the
.proto file for more information if you want to add
more layer data.

By the way, as Jeff's suggestion, use auto & MakeUnique<>()
to make the UniquePtr initialization more concise.
Attachment #8460085 - Attachment is obsolete: true
(In reply to Boris Chiou [:boris] from comment #17)
> Created attachment 8462422 [details] [diff] [review]
> Dump layer tree with layer scope on the viewer (v9)
> 
> We also want to dump layer tree on the viewer, so we
> can check the layer tree and layerscope together
> in the viewer. This can help us resolve more gfx bugs.
> 
> In this patch, I only add a part of the layer data to
> the protocol buffer packet, and you can check the
> .proto file for more information if you want to add
> more layer data.
> 
> By the way, as Jeff's suggestion, use auto & MakeUnique<>()
> to make the UniquePtr initialization more concise.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=a226cf7b9151
Attachment #8462422 - Flags: review?(dglastonbury)
Comment on attachment 8462422 [details] [diff] [review]
Dump layer tree with layer scope on the viewer (v9)

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

::: gfx/layers/composite/LayerManagerComposite.cpp
@@ +435,5 @@
> +
> +  // Dump to LayerScope Viewer
> +  if (LayerScope::CheckSendable()) {
> +    auto packet = MakeUnique<layerscope::Packet>();
> +    // Create a LayersPacket

Probably don't need a comment per line. You could just list the steps in a block before the function calls. Eg:

// Create a LayersPacket, dump Layers into it and transfer the 
// packet('s ownership) to LayerScope.

layerscope::LayersPacket* layersPacket = packet->mutable_layers();
this->Dump(layersPacket);
module...
Attachment #8462422 - Flags: review?(dglastonbury) → review+
(In reply to Dan Glastonbury :djg :kamidphish from comment #19)
> Comment on attachment 8462422 [details] [diff] [review]
> Dump layer tree with layer scope on the viewer (v9)
> 
> Review of attachment 8462422 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/composite/LayerManagerComposite.cpp
> @@ +435,5 @@
> > +
> > +  // Dump to LayerScope Viewer
> > +  if (LayerScope::CheckSendable()) {
> > +    auto packet = MakeUnique<layerscope::Packet>();
> > +    // Create a LayersPacket
> 
> Probably don't need a comment per line. You could just list the steps in a
> block before the function calls. Eg:
> 
> // Create a LayersPacket, dump Layers into it and transfer the 
> // packet('s ownership) to LayerScope.
> 
> layerscope::LayersPacket* layersPacket = packet->mutable_layers();
> this->Dump(layersPacket);
> module...

Thanks for your suggestion. I will revise it before checkin.
We also want to dump layer tree on the viewer, so we
can check the layer tree and layerscope together
in the viewer. This can help us resolve more gfx bugs.

In this patch, I only add a part of the layer data to
the protocol buffer packet, and you can check the
.proto file for more information if you want to add
more layer data.

By the way, as Jeff's suggestion, use auto & MakeUnique<>()
to make the UniquePtr initialization more concise.

Rebased and fix comments.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=037a22631e3e
Attachment #8462422 - Attachment is obsolete: true
Attachment #8463224 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/23d98e126c28
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.