Closed Bug 446100 Opened 16 years ago Closed 16 years ago

red lines rendering when zoomed in on google reader

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Platform:
x86
All
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: xenoterracide, Assigned: roc)

References

(
URL
)

Details

(Keywords: fixed1.9.1, regression)

Attachments

(2 files)

screenshot showing the red lines
309.34 KB, image/png
Details
possible work in progress
11.73 KB, patch
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008071805 (Gentoo) Firefox/3.0.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.1) Gecko/2008071805 (Gentoo) Firefox/3.0.1

attaching a screenshot that demonstrates. this didn't happen until I updated to 3.0.1

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

    
    

    
  
I get the same lines in Minefield.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1a1pre) Gecko/2008071702 Minefield/3.1a1pre ID:2008071702
Status: UNCONFIRMED → NEW
Ever confirmed: true

    
    

    
  
Regressed on Mozilla Central:
6-3 ok
6-4 broken

Looks like bug 433640 which is in both ranges.

Blocks: 433640

    
    

    
  
This reminds me of bug 421885.
URL: http://reader.google.com
Assignee: nobody → roc

    
    

    
  
It happens for example with https://bugzilla.mozilla.org/attachment.cgi?id=311143 which is in that bug.

    
    

    
  
Apparently, this is also a regression on the 1.9.0.x branch (see comment 3), so I guess it should also be considered if this should be fixed on that branch.
Flags: blocking1.9.0.2?

    
    

    
  
We should probably fix this in 1.9.0.x since we broke it there. However, 1.9.0.2 is too close for it to make. Roc, can you put this on your plate for 1.9.0.3?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.2?
Flags: blocking1.9.0.2-
Keywords: regression

    
    

    
  
I'd prefer dbaron to take it, but if he can't, I will.

    
    

    
  
Has anybody had a chance to make a simplified testcase?

    
    

    
  
See comment #7 --- there's a simplified testcase.
Flags: wanted1.9.1? → blocking1.9.1+
Priority: -- → P2

    
    

    
  
I'm a Google Reader engineer, and we've been getting some reports from our users about this problem. Is there an estimate of when this would be fixed by, so that we can investigate workarounds on our end if it won't be soon? Thanks.

    
    

    
  
someone should be able to look at this within a couple of weeks.

    
    

    
  
So, when zoomed, we're pixel-snapping to device pixels (which wasn't what I thought we were doing).  I need to think a bit about how that messes with the invariants we're trying to maintain.

    
    

    
  
The problem is that we compute the subimage rectangle using the pixel-snapped values. That's wrong; we should always compute the subimage rectangle using non-snapped values, because zooming and other transformations shouldn't change the set of pixels we sample.

    
    

    
  
(In reply to comment #16)
> non-snapped values, because zooming and other transformations shouldn't change
> the set of pixels we sample.

If you *really* want that invariant, then we have to snap to CSS pixels instead.

For example, that's the only thing that would make the pixels of the image used in this testcase:
  <div style="border: 10px solid; height: 5px; width: 5px">
    <img style="margin: -0.5px" height="10" width="10">
  </div>
invariant under 2x zooming.

But I'm not sure that's what we really want.  Maybe it is, though, although it wouldn't fully fix this bug in the non-integral zoom cases.

    
    

    
  
In an ideal output device, we'd sample the 6x5 top-left pixels of that image. We could snap to device pixels and still sample that same set of pixels.

It seems to me that that invariant is exactly what would fix this bug. The Web author should be thinking in terms of the ideal output without any snapping or device pixel limitations, so images and CSS should be designed so that no "red" pixels are sampled in that case. Then we should ensure that no matter what snapping or other rendering implementation we use, we don't sample any pixels that aren't sampled by the ideal model.

This stuff is too complicated and too fragile, so I'm trying to write down a list of requirements and work out from first principles what we should be doing.

    
    

    
  
OK, I think your requirements list has convinced me that the double-snapping I did is wrong.  The solution to this is probably to remove that.  (I think it was misguided anyway -- I was trying to preserve the invariant that an element is rendered the same no matter its subpixel position, but that invariant is already broken by child context that's not pixel-aligned.)

I'll try to write up a patch.

    
    

    
  


  
This stops doing the double-snapping, although it still leaves red lines at some (although I think fewer) zoom levels.  Still need to do more investigation.

It passes reftests, although I had to adjust one of the tests for bug 433640.  I think the adjustments make sense, although I'm not sure if they're exactly what we want.  See bug 433640 comment 37 for other comments on that test.  (I'm changing both the 31 and 31.1px cases.)

    
    

    
  
(Note that we do have significant test coverage here; I had to do a bit of debugging to get layout/reftests/pixel-rounding/background-image-tiling.html to pass.  That said, I'm sure there are some major gaps in that coverage as well.)

Speaking of which... I'm really not sure how to automate a test for *this* bug, since it's specific to zooming.  Any thoughts, or other ways to trigger it?  (I *think* it requires scaling that happens at the units level and not at the transform matrix level, since gfxContext::UserToDevicePixelSnapped won't snap if there's a transform.  But I'm a little puzzled, since I didn't have a problem with that when testing bug 449647.)

    
    

    
  
I filed bug 458487 for a systematic attack on all image-snapping bugs, with a proposal for how to do it.

(In reply to comment #22)
> Speaking of which... I'm really not sure how to automate a test for *this* bug,
> since it's specific to zooming.

Feel like adding zooming support to the reftest framework? I think that would be pretty useful.

    
    

    
  
So, I looked into what's happening when we're still seeing red lines at >100% zooms.

The first time is at zoom=133% (see the toolkit.zoomManager.zoomValues pref), where we have 45 app units per device pixel.  I'll look at horizontal coordinates only.  In the testcase, the element is at 10px offset relative to the canvas (8px of body padding, 2px of cellpadding), and its background position is at -32px (which is -22px relative to the canvas).  When we snap the background-clip area to the nearest device pixel, we move its origin 15 app units (1/3 of a device pixel, 1/4 of a CSS pixel) to the left.  When we snap the anchor to the nearest device pixel, we do exactly the opposite.  Thus the image column one pixel to the left of what was intended to be visible shows up.  I think this case had been fixed by the double-snapping in the past.

The second time is at zoom=170%, where we have 35 app units per device pixel.  This time we have red on the right edge.  We've snapped the background-clip area outwards 5 appunits in both directions (so its width is 10 appunits wider).  The anchor point snaps 10 appunits to the left, which means the distance between the anchor and the right edge of the element has increased by 15 appunits, which is apparently enough for us to see an extra pixel from the image.  (Note that the subimageRect that we pass to DrawTile, thanks to ScaleRoundOut, does have an extra pixel on the right, but it also had an extra pixel on the right in a number of cases where we didn't paint red.  I'm definitely suspicious of that ScaleRoundOut call, but I'm also not sure why it only causes problems some of the time, and I'd note that we also have red lines in the single image path (which doesn't have that call), although at different zoom levels.)
Flags: blocking1.9.0.4?

    
    

    
  
Is this the same thing happening with the odd lines (pink and cyan) in at least 3 areas when you zoom in using Gmail? The lines shift around, at one zoom level they're in one spot, another zoom level a different spot, and at a certain zoom level they disappear.

    
    

    
  
Should be fixed by bug 458487, which included tests for this bug.
Status: NEW → RESOLVED
Closed: 16 years ago
Depends on: 458487
Flags: in-testsuite+
Resolution: --- → FIXED
See Also:  → 1850752

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: