[LayerScope] Combine layer dump and layer scope

RESOLVED FIXED in mozilla34

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: u459114, Assigned: boris)

Tracking

(Blocks: 1 bug)

unspecified
mozilla34
x86_64
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 11 obsolete attachments)

1.08 MB, image/png
Details
179.28 KB, patch
boris
: review+
Details | Diff | Splinter Review
(Reporter)

Description

5 years ago
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.
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Updated

4 years ago
Assignee: mtseng → boris.chiou
Comment hidden (obsolete)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Updated

4 years ago
Depends on: 1035045
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Comment 15

4 years ago
Created attachment 8460085 [details] [diff] [review]
Dump layer tree with layer scope on the viewer (v8)

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
(Assignee)

Comment 16

4 years ago
Created attachment 8460088 [details]
LayerScope_with_LayerDump

The demo of this new viewer.
https://github.com/BorisChiou/layerscope/tree/layerdump
Attachment #8445010 - Attachment is obsolete: true
(Assignee)

Comment 17

4 years ago
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.
Attachment #8460085 - Attachment is obsolete: true
(Assignee)

Comment 18

4 years ago
(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
(Assignee)

Updated

4 years ago
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+
(Assignee)

Comment 20

4 years ago
(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.
(Assignee)

Comment 21

4 years ago
Created attachment 8463224 [details] [diff] [review]
Dump layer tree with layer scope on the viewer (v10, carry dglastonbury's r+)

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+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/23d98e126c28
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.