Community
Participate
Working Groups
Build Identifier: I'm breaking this out from bug 354435 for easier tracking. This corresponds to https://bugzilla.mozilla.org/show_bug.cgi?id=687861 There's a significant difference between horizontal scrolling speed in a textarea and in Orion. Reproducible: Always Steps to Reproduce: 1. Go to <https://bugzilla.mozilla.org/js/yui/yahoo-dom-event/yahoo-dom-event.js?1303753510>, select all, copy. 2. Go to scratchpad, select all, paste. 3. Scroll horizontally. 4. Now go to data:text/html,<textarea rows=10 cols=80 wrap=off>, paste. 5. Scroll horizontally. textarea scrolling is much faster and smoother.
I see it, we need to make the better.
We optimized horizontal scrolling to avoid unnecessary code that was done in updatePage() (like walking the DOM tree, resizing divs, etc). The code now only changes two style properties of the clientDiv (and overlayDiv): left and clip. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=fc7c768d8696bba80028b00a4498149efab09536 This change made a significant improvement in Chrome (see numbers below), but unfortunately it did not improve Firefox. It seems Firefox forces a reflow when the "clip" property changes. If we comment the code that changes the clip property in updatePage(), the times drop at least 2 times. Mihai, Do you know if a reflow should happen when the clip property is changed? I expected it not to, since it only affects painting (not layout). We added another performance bench to the demo to test this: - Open http://orion.eclipse.org/examples/textview/demo.html - Run the ScrollLeft performance test Here are the old and new times: Firefox 7.0.1 (Mac): 46000ms -> 46051ms (without clip: 18842ms) Firefox 7.0.1 (Win): 67501ms -> 60568ms (without clip: 22585ms) Chrome 15 (Mac): 19966ms -> 6374ms Chrome 15 (Win): 47096ms -> 12223ms
Hello Felipe/Silenio! :) (In reply to comment #2) > We optimized horizontal scrolling to avoid unnecessary code that was done in > updatePage() (like walking the DOM tree, resizing divs, etc). The code now > only changes two style properties of the clientDiv (and overlayDiv): left and > clip. > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=fc7c768d8696bba80028b00a4498149efab09536 This is very surprising for me. It is 90% the same code I worked on Friday. Even the hScrollOnly variable name and the approach you took. :) Last week I played with this bug - I also investigated what makes Orion scrolling slow (which parts of the code). I came to the same conclusion as you: it was all the stuff it does in updatePage(), but simply special-casing hScrollOnly was not enough. I was also down to just changing clipping. I didn't test other browsers so I didn't know how much perf improvement was in the changes I had. I just noticed I made horizontal scrolling a bit faster but far from enough. Anyway, perhaps the code is still slow for one or two reasons: 1. it still reads too many info from the page (scrollLeft/Top, clientWidth/Height, etc) 2. clipping and position changes themselves are perhaps slow. These are the two things I wanted to check, but didn't get to do so. I checked now and I have some results: - getScroll() and getClientWidth/Height() calls slow things down. - getScroll() causes most slow down. - if I pass the cached getScroll() result from handleScroll() down to updatePage() things go quite faster, but still not enough. - clipping is slow on Firefox. Webkit does fine, but not Gecko. :( - the only way to get übber-fast is to use scrollLeft for the clientDiv. Set overflow:hidden and set width and height to be that of the actual view. Then you can use scrollLeft to just change horizontal scrolling. No position change, no width changes, no clipping, etc. - this change makes scrolling instant in Firefox. - given the clientDiv and overlayDiv won't need width/height/left/top/clip updates vertical scrolling will also be faster. I have a very ugly WIP code that I used to test this theory today: https://github.com/mihaisucan/orion.client/tree/bug-358628-2 ... it's broken in Firefox and Chrome, but you can see horizontal scrolling is really fast in Firefox (it doesn't work in Chrome). It's just that the calculations are now semi-broken - I didn't spend time checking what values are needed for each property. :) Using scrollTop won't work for clientDiv because the element does not hold the entire code, of course. Still, in width the element holds the entire content, so it makes sense to use scrollLeft. (my broken WIP code linked above plays with scrollTop, but that's not really needed :), and it still needlessly updates clientDiv and overlayDiv width/height/left/top properties for vertical scrolling) It might make sense to set clientDiv.style.width = maxLineWidth if maxLineWidth > maxVisibleLineWidth, so the user can scroll beyond the width of the longest visible line. (in the above WIP the user can only scroll to the end of the longest visible line) Also, you might want to consider splitting out code that does actual scrolling from updatePage() into a separate method. That's what I ended up doing last week - updatePage is growing too big. I hope this helps. Thank you very much for your work! This is great stuff!
Is anyone doing further work on this? Or shall I take my WIP code further and make it work so that horizontal scrolling will be fast in Firefox as well? (we would want this fixed in time for for Firefox 11 alpha - that's about 4 weeks from now.)
(In reply to comment #4) > Is anyone doing further work on this? Or shall I take my WIP code further and > make it work so that horizontal scrolling will be fast in Firefox as well? > > (we would want this fixed in time for for Firefox 11 alpha - that's about 4 > weeks from now.) We are not working on this, since we were hoping that something could be done in Gecko to make changing the clipping faster. I do not think caching the scroll and client width/height will gain anything. I believe that scrolling horizontally by changing scrollLeft has future. But note that when you remove the clipping (and use overflow:hidden instead), you will break the margin around the editor content. It is not easy to see with the default settings (2px 1px), but try with this CSS: .view { background-color: white; padding: 10px 20px 30px 40px; background-color: red; } .viewContainer { background-color: #eeeeee; font-family: monospace; font-size: 10pt; } ::-webkit-scrollbar-corner { background-color: #eeeeee; } .viewContent { background-color: yellow; } Note that when scrolling towards the bottom, the top margin is gone.
(In reply to comment #2) > We optimized horizontal scrolling to avoid unnecessary code that was done in > updatePage() (like walking the DOM tree, resizing divs, etc). The code now > only changes two style properties of the clientDiv (and overlayDiv): left and > clip. > > http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=fc7c768d8696bba80028b00a4498149efab09536 Since this landed I see the currently highlighted line background color being cut off when I scroll horizontally. Can this problem be fixed?
(In reply to comment #6) > > Since this landed I see the currently highlighted line background color being > cut off when I scroll horizontally. > > Can this problem be fixed? Yes, we know about this problem. The current code is not changing the width for the clientDiv (to avoid reflow in the element). That is causing, only on Firefox, the clientDiv to clip the background (but not the foreground) for the line Div. Anyhow, we did some work using scrollLeft to scroll the content and using another div to implement the clipping (we longer using style.clip). We got far but we are still have some browser-specific problems that are preventing us for releasing the code. I should have an update about this problem still this week...
(In reply to comment #7) > (In reply to comment #6) > > > > Since this landed I see the currently highlighted line background color being > > cut off when I scroll horizontally. > > > > Can this problem be fixed? > > Yes, we know about this problem. > The current code is not changing the width for the clientDiv (to avoid reflow > in the element). That is causing, only on Firefox, the clientDiv to clip the > background (but not the foreground) for the line Div. > > Anyhow, we did some work using scrollLeft to scroll the content and using > another div to implement the clipping (we longer using style.clip). > We got far but we are still have some browser-specific problems that are > preventing us for releasing the code. > > I should have an update about this problem still this week... Great! Thanks for your continued work on fixing this bug. Much appreciated!
We finished implementing the strategy Felipe mentioned in comment#7. Now for FF only, horizontal scrolling is done by changing the scrollLeft offset of a clipDiv which provides the clipping necessary to have margins around the content. Setting scrollLeft on the clientDiv itself does not work because the background of lines will not draw up to the right edge of the client div when scrolled. It is also not possible to draw the bracket matching outline box on the edges of the client div since it has to be overflow:hidden. The code is only running for FF because it did not make any difference in performance for Chrome (already fast) and it did not work for IE. On IE, clicking on the vertical scrollbar thumb resets the scrollLeft offset of the clipDiv. We do not know why this happens. If we figure this out, we could have only one strategy for horizontal scrolling. By making the clientDiv position:absolute we started getting the resize handles of contentEditable. In order to fix this, we used the same strategy of bug#358623 of flippig the contentEditable state in mouse down/up. Fixed http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=925b8b8408973c9329f324dfef22c697ff505ddc
Hacks have been conditionally removed (FF version >= 13). http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/bundles?id=7b7ca9fd2c5338fd2c985b25162bd04b7be93b5d
Comment above was suppose to go in bug#372370.
Closing as part of a mass clean up of inactive bugs. Please reopen if this problem still occurs or is relevant to you. For more details see: https://dev.eclipse.org/mhonarc/lists/orion-dev/msg03444.html
