Overdraw should report dst pixels, not src pixels

RESOLVED FIXED in Firefox 34, Firefox OS v2.0

Status

()

Core
Graphics: Layers
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

Trunk
mozilla34
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

Comment 1

4 years ago
Created attachment 8471922 [details] [diff] [review]
transform src rect
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Attachment #8471922 - Flags: review?(jmuizelaar)
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.
Attachment #8471922 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 8471930 [details] [diff] [review]
patch

I added a comment, we can fix it when we hit these cases.
Attachment #8471922 - Attachment is obsolete: true
Attachment #8471930 - Flags: review+
Would mPixelsFilled += aRect.width * aRect.height * aTransform.Determinant() work for simple transforms?
(Assignee)

Comment 5

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

4 years ago
https://hg.mozilla.org/mozilla-central/rev/4ae9574c9834
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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.
(Assignee)

Comment 9

4 years ago
[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.
blocking-b2g: --- → 2.0?

Updated

4 years ago
blocking-b2g: 2.0? → 2.0+
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/cb40b8818fda
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
status-firefox32: --- → wontfix
status-firefox33: --- → wontfix
status-firefox34: --- → fixed
Duplicate of this bug: 1052742
You need to log in before you can comment on or make changes to this bug.