Rob Campbell [:rc] (:robcee) Reporter
	Description • 15 years ago

In Firefox version 3.6b3, install Feedly (available on AMO). Make sure you're using the "wide" view (make the browser wide enough so Feedly displays the sidebar with categories on the left). With Smooth Scrolling enabled, scroll slowly. See the screen dooring effect.

	Edwin Khodabakchian
	Comment 1 • 15 years ago

Hi Rob. 

Thank you very much for loging this bug. The scrolling performance issue in feedly 2.8 (version available in AMO) was caused by the use of -moz-box-shadow. You can see http://getsatisfaction.com/feedly/topics/feedly_1_2_scrolling_rendering_performance_issues_in_firefox_3_5_box_shadow for more details.

We have been able to work around this problem by not using -moz-box-shadow. Work around is included in the 2.9 version which is currently being reviewed by the AMO editor team.

Will right a blog post to warn other developers about the impact of -moz-box-shadow on scrolling until this bug get fixed.

Cheers,
Edwin

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 2 • 15 years ago

hi Ed,

We just tested this locally removing the -moz-box-shadow rules and still had the same scrolling issue. Jeff's current hunch is that it has something to do with the fixed side-panel. We'll post more as we find it.

	Jeff Muizelaar [:jrmuizel]
	Comment 3 • 15 years ago

I've put up a striped down version here:
http://people.mozilla.org/~jmuizelaar/p/feedly/

It looks like this has to do with the fixed text on the left. In Firefox 3.5 there is tearing at the bottom of the list, but in 3.6 the tearing happens over the entire list.

	Edwin Khodabakchian
	Comment 4 • 15 years ago

Hi Jeff. Nice to meet you. What do you mean by tearing? Is there anything we can do on our side by modifying the markup?

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 5 • 15 years ago

well, I think the point is actually fixing the rendering issue we're seeing with this particular markup rather than hiding it. I want my feedly to look good on windows in 3.6! :)

	Edwin Khodabakchian
	Comment 6 • 15 years ago

Understood. Please let us know if there is anything else we can do to help reduce the size of the test case (or anything else!)

	Jeff Muizelaar [:jrmuizel]
	Comment 7 • 15 years ago

The version at:
http://people.mozilla.org/~jmuizelaar/p/feedly/feedly2.html

makes the problem more obvious.

	Jeff Muizelaar [:jrmuizel]
	Comment 8 • 15 years ago

Ed,

Are the individual labels on the right side "position: fixed"?. If you make the whole block "position: fixed" the problem might go away.

	Edwin Khodabakchian
	Comment 9 • 15 years ago

Jeff.
The individual labels are not position: fixed. Only the feedlyTabs element is fixed.

#feedlyTabs 
{
color:#909090;
float:left;
font-size:12px;
font-weight:normal;
line-height:19px;
margin-left:-17px;
margin-top:44px;
position:fixed;
width:102px;
}

	Karl Tomlinson (:karlt)
	Comment 10 • 15 years ago

This looks like bug 529092.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 11 • 15 years ago

maybe it is, but the badness really springs from the fact that in some circumstances we're blitting like 86 separate rectangles for a single scroll operation. I'm still trying to figure out why.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 12 • 15 years ago

OK, so the scroll analysis in ComputeRepaintRegionForScroll finds that all the text isn't moving and needs to be repainted. (It doesn't really, because it's over a gray background, but the visibleRegionOfMovingContent is larger than it should be ... maybe I should fix that, but you could easily modify this testcase so that the text actually did need to be repainted.) This creates a complex aRepaintRegion, which leads to an even more complex aBlitRegion.

I suppose we should simplify aRepaintRegion outward whenever we can do that without adding much area to it.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 13 • 15 years ago

We probably do need to fix bug 529092 as well :-(

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 14 • 15 years ago

Edwin, can you file a separate bug about performance issues related to -moz-box-shadow, if you can reproduce the problem on trunk and haven't already filed such a bug? It's no doubt a separate issue.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 15 • 15 years ago

This patch reimplements nsRegion::SimplifyOutward to be a lot more intelligent. We coalesce rectangles in a way that tries to minimize the area that's added to the region. (It doesn't actually minimize, but it does pretty well.) Then we call SimplifyOutward on aRepaintRegion to bound the complexity of that region, which reduces the complexity of the blit region.

It reduces the screen-door effect on Windows, but doesn't eliminate it. Karl's right, we need to fix 529092.

	Karl Tomlinson (:karlt)
	Comment 16 • 15 years ago

(In reply to comment #14)
> Edwin, can you file a separate bug about performance issues related to
> -moz-box-shadow, if you can reproduce the problem on trunk and haven't already
> filed such a bug? It's no doubt a separate issue.

That might be bug 509052, which is filed for Mac, but looks like the same code is used for NT.

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 17 • 15 years ago

Can we get a blocking decision, here? Smooth scrolling isn't on by default, so I'm having a hard time saying that it's a hard release blocker, as opposed to a sucky one :(

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 18 • 15 years ago

it's definitely a regression from 1.9.1 which might help inform that decision.

	Edwin Khodabakchian
	Comment 19 • 15 years ago

If you guys decide not to fix it, is there a work around?

	Edwin Khodabakchian
	Comment 20 • 15 years ago

Note: Robert: Karl is right the -moz-box-shadow part of the bug appears on both windows and mac and is identical to bug 509052. I will update that bug with the safari information you requested.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 21 • 15 years ago

I think you could work around it by giving the position:fixed element a solid background.

	Edwin Khodabakchian
	Comment 22 • 15 years ago

Hi Robert. Tried the "solid background" work around using 3.6 beta 3 and it does not seem to make a difference. Does it depend on the region simplification? Should I try it on another build?

	Mike Beltzner [:beltzner, not reading bugmail]
	Comment 23 • 15 years ago

Not blocking as it's not a default option, do want a fix, though.

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 24 • 15 years ago

I've been using Feedly on my windows PC this morning and I have noticed that this issue is present without autoscrolling enabled. We may want to reconsider blocking status.

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 25 • 15 years ago

correction: without _smooth_scrolling enabled.

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 26 • 15 years ago

OK, I've got a fix for bug 529092 which makes this go away.

	Edwin Khodabakchian
	Comment 27 • 15 years ago

Thank you very much Robert. Next time you are in the bay area, drinks will be on us!

	Robert O'Callahan (:roc) (email my personal email if necessary) Assignee
	Comment 28 • 15 years ago

Fixed by checkin for bug 529092.

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 29 • 15 years ago

marking fixed.

	Rob Campbell [:rc] (:robcee) Reporter
	Comment 30 • 15 years ago

and verified in Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2b5pre) Gecko/20091201 Namoroka/3.6b5pre (.NET CLR 3.5.30729) ID:20091201044929

thanks!