1170966 - Check quad's effective region before drawing

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jerry Shih[:jerry] (UTC+8) (inactive)]

With tiling, I see that we draw a lot of quads which are not shown on screen. This bug will try to check each quad's region and do the culling if needs.

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: check quad's effective region before drawing. v1

This patch check the quad's effective region with the target buffer size. If there is no overlapping area, we can skip this function. With this, we can improve the layerscope tool and the tiling layer rendering performance slightly.

Comment 2

[Nicolas Silva [:nical]]

Comment on attachment 8614701 [details] [diff] [review]
check quad's effective region before drawing. v1

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

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +953,5 @@
>      return;
>    }
> +
> +  // XXX: This doesn't handle 3D transforms. It also doesn't handled rotated
> +  //      quads. Fix me.

It's okay that mPixelsFilled has some imprecision in those cases but we should not do this culling optimization if we have a rotation and/or a 3D transform. You should check that !aTransform.HasNonAxisAlignedTransform() before culling out the quad. That should still cover most of the cases you are interested in.

Comment 3

[Nicolas Silva [:nical]]

Comment on attachment 8614701 [details] [diff] [review]
check quad's effective region before drawing. v1

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

Sorry, TransformBounds will give a conservatively big rect, so it's correct.

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try link:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3859df82bb24

Comment 5

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Please land the attachment 8614701 [details] [diff] [review] to m-c

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/89807785d4d4

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Backed out for OSX 10.10 reftest failures.
https://treeherder.mozilla.org/logviewer.html#?job_id=10481451&repo=mozilla-inbound

Comment 8

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab595285aa97

Comment 9

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: check quad's effective region before drawing. v2

Inflate a small size to avoid the test failed at mac 10.10.
https://treeherder.mozilla.org/logviewer.html#?job_id=10481451&repo=mozilla-inbound

Comment 10

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8617066 [details] [diff] [review]
check quad's effective region before drawing. v2

pass the try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec6c0dd55353

Comment 11

[Nicolas Silva [:nical]]

Comment on attachment 8617066 [details] [diff] [review]
check quad's effective region before drawing. v2

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

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +958,5 @@
> +  Rect destRect = aTransform.TransformBounds(aRect);
> +  mPixelsFilled += destRect.width * destRect.height;
> +
> +  // Do a simple culling if this rect is out of target buffer.
> +  // Inflate a small size to avoid some corner edge hit test.

I think you meant "corner case" or "edge case".

Comment 12

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: check quad's effective region before drawing. v3, r=nical

update comment.

Comment 13

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8617463 [details] [diff] [review]
check quad's effective region before drawing. v3, r=nical

Please land this patch to m-c.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7674044400c8

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7674044400c8