Closed Bug 1212664 Opened 9 years ago Closed 9 years ago

Compositor surface dumping code performs y-inversion incorrectly

Categories

(Core :: Graphics: Layers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(1 file)

The transform set in YInvertImageSurface() is:

    Matrix::Scaling(1.0, -1.0) *
    Matrix::Translation(0.0, aSurf->GetSize().height)

That's ends up being

    1  0     0
    0 -1  -height

which is incorrect because after the -1 y-scale flips the y-coordinate, translating by -height moves the pixel further off-surface. The matrix we want is

    1  0     0
    0 -1   height

which is obtained as:

    Matrix::Translation(0.0, aSurf->GetSize().height) *
    Matrix::Scaling(1.0, -1.0)
Bug 1212664 - Perform y-inversion correctly when dumping compositor surface. r=jrmuizel
Attachment #8671097 - Flags: review?(jmuizelaar)
Comment on attachment 8671097 [details]
MozReview Request: Bug 1212664 - Perform y-inversion correctly when dumping compositor surface. r=jrmuizel

Jeff gave me verbal r+.
Attachment #8671097 - Flags: review?(jmuizelaar) → review+
(In reply to Botond Ballo [:botond] from comment #0)
> The transform set in YInvertImageSurface() is:
> 
>     Matrix::Scaling(1.0, -1.0) *
>     Matrix::Translation(0.0, aSurf->GetSize().height)
> 
> That's ends up being
> 
>     1  0     0
>     0 -1  -height
> 
> which is incorrect because after the -1 y-scale flips the y-coordinate,
> translating by -height moves the pixel further off-surface. The matrix we
> want is
> 
>     1  0     0
>     0 -1   height
> 
> which is obtained as:
> 
>     Matrix::Translation(0.0, aSurf->GetSize().height) *
>     Matrix::Scaling(1.0, -1.0)

Sorry I mixed those up.

     Matrix::Translation(0.0, aSurf->GetSize().height) *
     Matrix::Scaling(1.0, -1.0)

is the one we had, which gives the incorrect matrix

     1  0     0
     0 -1  -height
I should also mention that the Part 7 patch in bug 709490 (which recently landed and got backed out) contains this fix, though it's not clear whether that's intentional or it just snuck into the patch.
See Also: → 709490
https://hg.mozilla.org/mozilla-central/rev/d9a501cd1b1d
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: