703231 - [Mac] High quality down sampling causes scan lines while decoding images

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect)

Comment 1

[Benoit Girard (:BenWa)]

Test case: http://infidigm.net/scale.html

Comment 2

[Benoit Girard (:BenWa)]

Attachment: patch

Comment 3

[Joe Drew (not getting mail)]

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

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

::: image/src/Decoder.cpp
@@ +190,5 @@
> +#ifdef XP_MACOSX
> +    // Bug 703231
> +    // Because of high quality down sampling on mac we show scan lines while decoding.
> +    // Bypass this problem by redrawing the border.
> +    mInvalidRect = nsIntRect(mInvalidRect.x-1, mInvalidRect.y-1, mInvalidRect.width+2, mInvalidRect.height+2);

Use .Inflate(1) instead, and test what ends up happening if we invalidate (-1, -1) to (width, height).

Comment 4

[Benoit Girard (:BenWa)]

Attachment: patch v2

Non trivial change, re-requesting review.

Comment 5

[Joe Drew (not getting mail)]

Comment on attachment 575144 [details] [diff] [review]
patch v2

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

::: image/src/Decoder.cpp
@@ +198,5 @@
> +    mImage.GetHeight(&height);
> +    nsIntRect mImageBound(0, 0, width, height);
> +
> +    mInvalidRect.Inflate(1);
> +    mInvalidRect = mInvalidRect.Union(mImageBound);

Don't you want to intersect here?

Comment 6

[Benoit Girard (:BenWa)]

Attachment: patch v3

Comment 7

[Joe Drew (not getting mail)]

Comment on attachment 575381 [details] [diff] [review]
patch v3

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

It'd be handy to have a reftest here. Might be a little tricky, but worthwhile.

Comment 8

[Benoit Girard (:BenWa)]

Do we have any dupes of this bug? If we don't I find this concerning.

Do you have any ideas how I can capture this in the reftest? The current behavior is timing dependent. Have we written similar reftests in the past?

Comment 9

[Joe Drew (not getting mail)]

You'll have to write an HTTP reftest, similar to the one in layout/reftests/backgrounds/delay-image-response.sjs, but which sends only part of the image, delays a little bit, and then sends the rest.

Comment 10

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a31b251053fb

Comment 11

[Mounir Lamouri (:mounir)]

https://hg.mozilla.org/mozilla-central/rev/a31b251053fb

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

I'm still seeing this with the current Nightly 11.0a1 (2011-12-04) when opening the URL from bug 705224.

Comment 16

[Benoit Girard (:BenWa)]

I have noticed it too. Should we be more aggressive with our repainting area. Do we know what the bound should be?

Comment 17

[Joe Drew (not getting mail)]

We need to have one screen-space pixel be redrawn. So if you can get the inverse transform, then use it to transform 1 unit height into image space, you can use that value instead of just 1 pixel.

Comment 18

[Benoit Girard (:BenWa)]

I don't see this anymore.

Comment 19

[Giorgio Borgonovo]

I have the same problem in Firefox 29.0 under Kubuntu 14.04 (fresh install).