Fading resized images with HTML <img> width/height attributes changes the image rendering

NEW
Unassigned
(NeedInfo from)

Status

()

Core
Layout
5 years ago
8 months ago

People

(Reporter: Loic, Unassigned, NeedInfo)

Tracking

({regression})

Trunk
x86_64
Windows 7
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: workaround in comment 0)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
Build ID: 20120312181643

Steps to reproduce:

I think it's a regression in how Firefox renders images with opacity animation, especially when the images has been resized with HTML <img> width/height attributes (usually by more than half).
The effect can be very subtle like a slight off-setting (~1 px) or a kind of antialiasing.

STR:
1. Open the testcase I joined
2. Move the mouse over the images to animate the opacity
3. Try at different zoom levels to observe the off-setting/antialiasing

WORKAROUND: adding "box-shadow: #000 0em 0em 0em;" to images fixes the bad rendering.

I tried mozregression and I found there is build range (from 2011-10-27 to 2011-11-08) whithin I can't observe the bad rendering because there is another bug (regression) in zoom or opacity (I don't know exactly).

So I searched the 1st build from which the bad image rendering is visible AND the bug in zoom/opacity is fixed. And I guess it's in the bugfix about zoom/opacity that there is a regression causing the bad rendering:

m-c
good=2011-11-08
bad=2011-11-09
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=81dedcc49ac0&tochange=6d42793c4807

m-i
good=2011-11-07
bad=2011-11-08
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=60adc85c97be&tochange=a62abefaab90


For the record, I add too the mozregression about the bug in zoom/opacity, maybe it can help to find the regression:

m-c
good=2011-10-26
bad=2011-10-27
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc66accc8181&tochange=4bb7c983d589

m-i
good=2011-10-25
bad=2011-10-26
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a414fc0d982b&tochange=ebd6501de883
(Reporter)

Comment 1

5 years ago
Created attachment 615171 [details]
Bug_745549_testcase
(Reporter)

Comment 2

5 years ago
Comment on attachment 615171 [details]
Bug_745549_testcase

><!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
><html>
><head>
><meta charset="utf-8">
><title>Bug 745549 - Testcase</title>
><style type="text/css">
>body
>{
>    background-color: gray; 
>}
>.gallery-row img
>{
>    //box-shadow: #000 0em 0em 0em;  //workaround: adding box-shadow fixes the bad rendering
>    overflow: hidden;
>    float: left;      
>}
>.clear
>{
>    clear: both;
>}  
></style>
></head>
><body>
><div class='gallery-row'>
>     <img src='http://i.imgur.com/C5SgX.jpg' width='324' height='105' alt='fantasy'/>
>     <img src='http://i.imgur.com/HywIc.jpg' width='271' height='209' alt='statues'/>
>     <div class='clear'></div>
></div>
><script src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js"></script>
><script>
>$('.gallery-row img').fadeTo(100,.3).on("mouseenter", function(){
>    $(this).stop().fadeTo(100,1);
>}).on("mouseout", function(){
>    $(this).stop().fadeTo(100,.3);
>});
></script>
></body>
></html>
Attachment #615171 - Attachment description: Bug745549_testcase → Bug_745549_testcase
Attachment #615171 - Attachment mime type: text/plain → text/html
(Reporter)

Updated

5 years ago
Keywords: regression

Comment 3

5 years ago
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=98013fe19dcb&tochange=d7f3bfc7cd46
Triggered by;
8b89d7037306	Matt Woodrow — Bug 695275 - Fix conversion of ThebesLayers to ImageLayers. r=roc

And it seems to be fixed by Bug 698590

Comment 4

5 years ago
Oops please ignore comment #3
(Reporter)

Comment 5

5 years ago
The bug is present in FF10+.

Comment 6

5 years ago
1st regression: positioning error
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=98013fe19dcb&tochange=d7f3bfc7cd46
Triggered by;
8b89d7037306	Matt Woodrow — Bug 695275 - Fix conversion of ThebesLayers to ImageLayers. r=roc

2nd regression :rendering issue(fixed positioning but broken antialiasing)
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ebb4fe2d4c83&tochange=bcf9c9ec97a0
Triggered by;
95efc21bf5af	Matt Woodrow — Bug 698590 - Make ConfigureLayer use DevPixels instead of AppPixels. r=roc
Blocks: 698590, 695275
Status: UNCONFIRMED → NEW
Component: Untriaged → Layout
Ever confirmed: true
Product: Firefox → Core
QA Contact: untriaged → layout
(Reporter)

Updated

5 years ago
Whiteboard: workaround in comment 0

Comment 7

5 years ago
I have filed the same bug as Bug 745446 - Image rendering varies with opacity on resized images.  My example can be seen here - http://bobtipping.com/fftest/low.html
I think this is the result of different resampling algorithms being used. When opacity is animated, we are transforming the image surface using a transform on the ImageLayer and the resampling is happening in the GPU texture fetcher. Otherwise, we're using whatever the 2D drawing code does.

I'm not sure what we can do here. Jeff, Bas?
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> I think this is the result of different resampling algorithms being used.
> When opacity is animated, we are transforming the image surface using a
> transform on the ImageLayer and the resampling is happening in the GPU
> texture fetcher. Otherwise, we're using whatever the 2D drawing code does.
> 
> I'm not sure what we can do here. Jeff, Bas?

Nothing jumps to mind. I'll think about it for a bit.

Comment 10

5 years ago
I am pretty sure this bug did not exist before FF10 what changed ?

Updated

5 years ago
Duplicate of this bug: 745446
> I am pretty sure this bug did not exist before FF10 what changed ?

Please see comment 6.  The bug fixes that caused this landed for Firefox 10.
Version: 11 Branch → Trunk
(Reporter)

Updated

5 years ago
Duplicate of this bug: 776575
(Reporter)

Updated

5 years ago
Duplicate of this bug: 780521

Comment 15

4 years ago
I have just noticed this bug, and it does not happen when the width attribute is an odd number. Just something to look at.

As you can see in this example, the bug happens to image #2 and #4.
(Reporter)

Comment 16

4 years ago
Which example?

Comment 17

4 years ago
Oops sry, http://jsfiddle.net/UtDSs/1/

Now apparently it's a comination of the pixels inside a div as well. in other tests, some pictures with even number didn't show the same error, so my believe is that it could be the combination of pixels of all the pictures.

Comment 18

4 years ago
(In reply to gui from comment #15)
> I have just noticed this bug, and it does not happen when the width
> attribute is an odd number. Just something to look at.
> 
> As you can see in this example, the bug happens to image #2 and #4.

http://jsfiddle.net/UtDSs/1/

Comment 19

4 years ago
This is an annoying bug, especially for image galleries making use of opacity animation. A popular one is Galleria.io. There, you basically cannot use the 'fade' transition type in Firefox as a result of this bug. Reference: http://support.galleria.io/discussions/problems/2549-minor-but-extremely-annoying-image-resize-artifact-after-fade-transition-on-firefox

Comment 20

4 years ago
Here is the solution: http://nickpye.com/2013/04/03/css3-opacity-transition-image-wiggle-bug-in-mozilla-firefox/
I had to add "-moz-backface-visibility: hidden" to solve the problem;

Comment 21

4 years ago
Created attachment 826317 [details]
new testcase

just to contribute with a new testcase, the most interesting one in the zip attached is #02 where there is nothing different compared to #01 except the image is repeated and this seems to switch something in the rendering, no distortion of the image.
(Reporter)

Updated

4 years ago
Attachment #826317 - Attachment mime type: text/plain → application/zip
This seems likely to be a difference between the HQ scaling codepath and layers GPU scaling. Probably the issue is that a layers ImageContainer does not contain the HQ scaled version of the image, it contains the unscaled version.

It's possible that a similar issue could start to bite us very soon once we have downscale-during-decode, because then we won't *have* an unscaled version. We can obviously just place the HQ scaled version in the ImageContainer, which would solve this bug, but then how will we trigger a new decode if we start upscaling the image layer?

I suspect we need a combination of things here: we should probably decode to the 'real' size of the image if the 'will-change' property is present, and we need some way for the layer system to let the image know that we will be upscaling (maybe 'GetImageContainer' needs to take a size parameter, but I'm not sure how frequently that's called right now).

Matt, what do you think about all this? I'm also needinfo'ing myself, because while I can't tackle this immediately I want to come back to it when the time is right.
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(seth)
(In reply to Seth Fowler [:seth] from comment #22)
> This seems likely to be a difference between the HQ scaling codepath and
> layers GPU scaling. Probably the issue is that a layers ImageContainer does
> not contain the HQ scaled version of the image, it contains the unscaled
> version.

Yes, that's almost certainly the issue here.

> 
> It's possible that a similar issue could start to bite us very soon once we
> have downscale-during-decode, because then we won't *have* an unscaled
> version. We can obviously just place the HQ scaled version in the
> ImageContainer, which would solve this bug, but then how will we trigger a
> new decode if we start upscaling the image layer?
> 
> I suspect we need a combination of things here: we should probably decode to
> the 'real' size of the image if the 'will-change' property is present, and
> we need some way for the layer system to let the image know that we will be
> upscaling (maybe 'GetImageContainer' needs to take a size parameter, but I'm
> not sure how frequently that's called right now).

Yeah I agree, I think we need GetImageContainer to estimate the final size of the image so we can pick an appropriately scaled one.

We call GetImageContainer every frame (on the client side) so this would be suitable. The total transform on the ImageLayer (base transform + post scale) should be enough to know what size image to pick.
Flags: needinfo?(matt.woodrow)
(Reporter)

Comment 24

3 years ago
There is an improvement in the the latest Nightly.

Comment 25

3 years ago
Wow, thank you so much for this workaround.  It was driving me nuts and didn't see this fix anywhere else.  I tried the backface property and that didn't help, but the box shadow worked like a charm!!!
You need to log in before you can comment on or make changes to this bug.