Open Bug 745549 Opened 12 years ago Updated 2 years ago

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

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Platform:
x86_64
Windows 7
Type:
defect

Tracking

()

People

(Reporter: epinal99-bugzilla2, Unassigned)

References

Details

(Keywords: regression, Whiteboard: workaround in comment 0)

Attachments

(2 files)

Bug_745549_testcase
1023 bytes, text/html
Details
new testcase
425.72 KB, application/zip
Details 
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
Attached file Bug_745549_testcase — 

    
    

    
  
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

    
  
Keywords: regression

    
    

    
  
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

    
    

    
  
Oops please ignore comment #3

    
    

    
  
The bug is present in FF10+.

    
    

    
  
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

    
  
Whiteboard: workaround in comment 0

    
    

    
  
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.

    
    

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

    
    

    
  
> 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

    
    

    
  
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.

    
    

    
  
Which example?

    
    

    
  
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.

    
    

    
  
(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/

    
    

    
  
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

    
    

    
  
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;

    
    

    
  

      
      
      
      

        Attached file
          
        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.

    
  

        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)

    
    

    
  
There is an improvement in the the latest Nightly.

    
    

    
  
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!!!

    
  
Severity: normal → S3

    
    

    
  
The severity field for this bug is relatively low, S3. However, the bug has 3 duplicates.

:dholbert, could you consider increasing the bug severity?


For more information, please visit auto_nag documentation.


Flags: needinfo?(dholbert)

    
    

    
  
The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.


Flags: needinfo?(dholbert)

    
    

    
  
Clear a needinfo that is pending on an inactive user.


Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.


For more information, please visit auto_nag documentation.


Flags: needinfo?(seth.bugzilla)

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: