Closed Bug 728352 Opened 10 years ago Closed 10 years ago

Implement a render trace utility for OMTC debugging

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: BenWa, Assigned: BenWa)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch RenderTrace (obsolete) — Splinter Review
This works well on the desktop but we will need some follow up patches on mobile. I don't think this will handle layers with 3D transforms correctly. It also doesn't get the right offsets for temporary layer managers. Best way to go is to fix this up incrementally.

Demo:
http://people.mozilla.org/~bgirard/rendertrace.html
Attached patch Rendertrace (obsolete) — Splinter Review
Assignee: nobody → bgirard
Attachment #598324 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #598329 - Flags: review?(matt.woodrow)
Comment on attachment 598329 [details] [diff] [review]
Rendertrace

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

::: gfx/layers/RenderTrace.cpp
@@ +58,5 @@
> +    };
> +
> +static gfx3DMatrix GetRootTransform(Layer *aLayer) {
> +  if (aLayer->GetParent() != NULL)
> +    return GetRootTransform(aLayer->GetParent()) * aLayer->GetTransform();

ProjectTo2D here too

@@ +66,5 @@
> +void RenderTraceLayers(Layer *aLayer, const char *aColor, gfx3DMatrix aRootTransform, bool aReset) {
> +  if (!aLayer)
> +    return;
> +
> +  gfx3DMatrix trans = aRootTransform * aLayer->GetTransform();

Call ProjectTo2D on the GetTransform() result before multiplying.

@@ +67,5 @@
> +  if (!aLayer)
> +    return;
> +
> +  gfx3DMatrix trans = aRootTransform * aLayer->GetTransform();
> +  nsIntRect clipRect = aLayer->GetEffectiveVisibleRegion().GetBounds();

Convert to gfxRect

@@ +68,5 @@
> +    return;
> +
> +  gfx3DMatrix trans = aRootTransform * aLayer->GetTransform();
> +  nsIntRect clipRect = aLayer->GetEffectiveVisibleRegion().GetBounds();
> +  clipRect.MoveBy((int)trans.ProjectTo2D()[3][0], (int)trans.ProjectTo2D()[3][1]);

trans.TransformBounds(gfxRect)

@@ +71,5 @@
> +  nsIntRect clipRect = aLayer->GetEffectiveVisibleRegion().GetBounds();
> +  clipRect.MoveBy((int)trans.ProjectTo2D()[3][0], (int)trans.ProjectTo2D()[3][1]);
> +
> +  printf_stderr("%s RENDERTRACE %u rect #%s%s %i %i %i %i\n",
> +    layerName[aLayer->GetType()], (int)PR_IntervalNow(),

Layers already have Name(), is that not enough?

@@ +72,5 @@
> +  clipRect.MoveBy((int)trans.ProjectTo2D()[3][0], (int)trans.ProjectTo2D()[3][1]);
> +
> +  printf_stderr("%s RENDERTRACE %u rect #%s%s %i %i %i %i\n",
> +    layerName[aLayer->GetType()], (int)PR_IntervalNow(),
> +    colors[colorId%19], aColor,

These colors are confusing. Why do we need two? Having the input parameter as a gfxRGBA would make a lot more sense here. Converting that to an HTML format color string should be easy.

@@ +85,5 @@
> +
> +  if (aReset) colorId = 0;
> +}
> +
> +void RenderTraceInvalidateStart(Layer *aLayer, const char *aColor, nsIntRect aRect) {

const nsIntRect&, modifying parameters in-place is ew. :)

@@ +87,5 @@
> +}
> +
> +void RenderTraceInvalidateStart(Layer *aLayer, const char *aColor, nsIntRect aRect) {
> +  gfx3DMatrix trans = GetRootTransform(aLayer);
> +  aRect.MoveBy((int)trans.ProjectTo2D()[3][0], (int)trans.ProjectTo2D()[3][1]);

TransformBounds
Attached patch Rendertrace (obsolete) — Splinter Review
The reason I made the color as string is because I give an ID to each layer in the R channel, so the parameters are for the GB channels only. The frontend assumes keys all the information by layer color so they must be unique so we need a different RGB color code for each layer. We will likely want to improve this in the near future.
Attachment #598329 - Attachment is obsolete: true
Attachment #600268 - Flags: review?(matt.woodrow)
Attachment #598329 - Flags: review?(matt.woodrow)
I forgot to remove color[], I will remove it before landing.
Comment on attachment 600268 [details] [diff] [review]
Rendertrace

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

Still don't love the string colors, but should be fine for now

::: gfx/layers/RenderTrace.cpp
@@ +57,5 @@
> +  gfx3DMatrix layerTrans = aLayer->GetTransform();
> +  if (aLayer->GetParent() != NULL) {
> +    return GetRootTransform(aLayer->GetParent()) * layerTrans.ProjectTo2D();
> +  }
> +  return layerTrans.ProjectTo2D();

Move both of these ProjectTo2D calls up to above the if condition?

@@ +64,5 @@
> +void RenderTraceLayers(Layer *aLayer, const char *aColor, const gfx3DMatrix aRootTransform, bool aReset) {
> +  if (!aLayer)
> +    return;
> +
> +  gfx3DMatrix trans = aRootTransform * aLayer->GetTransform();

ProjectTo2D() here too
Attachment #600268 - Flags: review?(matt.woodrow) → review+
Attached patch RendertraceSplinter Review
Carrying forward r+

I agree let's improve the layer color in a follow up.
Attachment #600268 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c155fde4c0c7
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.