1052857 - Overdraw should report dst pixels, not src pixels

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Benoit Girard (:BenWa)]

This is noticeable in the camera app where the video preview doesn't (cant?) match the destination 1:1.

Comment 1

[Benoit Girard (:BenWa)]

Attachment: transform src rect

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8471922 [details] [diff] [review]
transform src rect

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

::: gfx/layers/opengl/CompositorOGL.cpp
@@ +1068,5 @@
>  
> +  {
> +    const Rect destRect = aTransform.TransformBounds(aRect);
> +    mPixelsFilled += destRect.width * destRect.height;
> +  }

This doesn't really compute the right thing because of TransformBounds.

Comment 3

[Benoit Girard (:BenWa)]

Attachment: patch

I added a comment, we can fix it when we hit these cases.

Comment 4

[Markus Stange [:mstange]]

Would mPixelsFilled += aRect.width * aRect.height * aTransform.Determinant() work for simple transforms?

Comment 5

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ae9574c9834

(In reply to Markus Stange [:mstange] from comment #4)
> Would mPixelsFilled += aRect.width * aRect.height * aTransform.Determinant()
> work for simple transforms?

My linear algebra is very poor so I don't know. If it's better then this please amend this patch. If not I'll look into it tomorrow.

Comment 6

[Markus Stange [:mstange]]

My linear algebra is very poor, too, I was just curious if you knew. Don't worry about it. What you landed is fine I think.

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4ae9574c9834

Comment 8

[Justin D'Arcangelo [:justindarc]]

Comment on attachment 8471930 [details] [diff] [review]
patch

Camera - Flame (KK)
-------------------
Photo: 392
Video: 542
Recording: 529

Camera - Flame (KK) WITH PATCH
------------------------------
Photo: 450
Video: 417
Recording: 404

So, overdraw *increases* slightly with this patch for the regular photo viewfinder. However, it goes down significantly for the video viewfinder and while recording video.

Comment 9

[Benoit Girard (:BenWa)]

[Blocking Requested - why for this release]: This isn't really required, this change has no user visible impact and is very unlikely to causes stability issues. However if we compare overdraw with 2.0 and master the numbers will be wrong and it might lead us to chasing down ghosts so I rather uplift this.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/cb40b8818fda