Scrolling worse on macOS Sierra

[Neil Hodgson]

The performance of scrolling appears choppier on macOS Sierra than on El Capitan using a 2013 MacBook Pro Retina.

The main problem appears to be adjusting the scroll position to scroll by whole lines which appears to be more aggressive. Its more difficult to scroll by a line or two. Turning this off leads to smoother scrolling but still not quite as good as El Capitan. Most noticeable in wide and complex parts of files.

There will likely be bugs if the whole line adjustment is turned off as Scintilla was never designed for this.

Neil

[Chinh Nguyen]

I've had a few users complain about this and I never could reproduce it until a user sent a video. What I didn't realize the user was doing was scrolling very slowly or like you said, by a line or two.

I've started to investigate this issue and discovered something weird. If I use my own SciContentView subclass with my own scrollWheel: method that simply calls [super scrollWheel:theEvent] (SCROLL_WHEEL_MAGNIFICATION is undefined), the vertical scrolling is better for precise scrolling devices like the magic mouse and trackpad. You can test this for yourself by inserting the following into cocoa/ScintillaTest/AppController.mm:

@interface MyContentView : SCIContentView
@end


@implementation MyContentView
- (void) scrollWheel: (NSEvent *) theEvent
{
 [super scrollWheel:theEvent];
}
@end


@interface MyScintillaView : ScintillaView
@end


@implementation MyScintillaView
+ (Class)contentViewClass
{
 return([MyContentView class]);
}
@end

and changing the scintilla editor creation to use MyScintillaView.

For non-Apple mice with scrollwheels (hasPreciseScrollingDeltas==0), no scrolling happens if you scroll a "notch" at a time. I fixed that by scrolling by lineHeight when scrollingDeltaY is less than 1.

[Chinh Nguyen]

I fixed that by scrolling by lineHeight when scrollingDeltaY is less than 1.

Actually, when it's less than lineHeight.

[Neil Hodgson]

Chinh Nguyen:

> I've started to investigate this issue and discovered something weird. If I use my own SciContentView subclass with my own scrollWheel: method that simply calls [super scrollWheel:theEvent] (SCROLL_WHEEL_MAGNIFICATION is undefined), the vertical scrolling is better for precise scrolling devices like the magic mouse and trackpad.

When I try a similar change it "causes visual garbage to appear when scrolling horizontally (on OS X 10.9) with a retina display” as mentioned in the comment although this is on current macOS 10.12.6.

Do you see small pieces of garbage when horizontally scrolling? They appear to be debris from characters being scrolled and could indicate a problem with clipping.

Neil

[Neil Hodgson]

Me:

> When I try a similar change it "causes visual garbage to appear when scrolling horizontally (on OS X 10.9) with a retina display” as mentioned in the comment although this is on current macOS 10.12.6.

Here is an image - examine “means” on the first line:
http://www.scintilla.org/ScrollDebris.png

Neil

[Chinh Nguyen]

I don't develop on a retina display so I wouldn't have seen the visual garbage but I did see it once I tried it on a retina display. However, I was able to fix the visual garbage from horizontal scrolling by adding the following to the adjustScroll: method in ScintillaView.mm:

  if ((rc.origin.x > 0) && (NSMaxX(rc) < contentRect.size.width)) {
    // Only snap for positions inside the document - allow outside
    // for overshoot.
    rc.origin.x = roundf(static_cast<XYPOSITION>(rc.origin.x));
  }

[Neil Hodgson]

Chinh Nguyen:

> I don't develop on a retina display so I wouldn't have seen the visual garbage but I did see it once I tried it on a retina display. However, I was able to fix the visual garbage from horizontal scrolling by adding the following to the adjustScroll: method in ScintillaView.mm:

OK, that seems better. Attached a patch that changes ScintillaView which makes vertical scrolling work better and avoids the garbage.

It also allows zooming with Command+Mouse Scroll as originally occurred. That could be made conditional if anyone objects.

Neil

[Chinh Nguyen]

I personally don't use the scroll wheel magnification because pressing command-W to close the window while its content is still scrolling due to inertia scrolling will unintentionally change the magnification. For me, the magnification is a setting that's automatically saved so I'll get a really big or really small font size for the next window I open.

FWIW, here's my SCIContentView subclass override for scrollWheel: to support non-Apple devices:

- (void)scrollWheel:(NSEvent *)theEvent
{
 NSScrollView *scrollView = self.enclosingScrollView;


 if (scrollView && !theEvent.hasPreciseScrollingDeltas) {
 NSClipView *clipView = scrollView.contentView;
 CGFloat scrollingDeltaY = MAX(fabs(theEvent.scrollingDeltaY), scrollView.verticalLineScroll);
 NSRect scrollRect = NSOffsetRect(clipView.documentVisibleRect, -theEvent.scrollingDeltaX , theEvent.scrollingDeltaY < 0. ? scrollingDeltaY : -scrollingDeltaY);
 [clipView.documentView scrollRectToVisible:scrollRect];
 }
 else {
 [super scrollWheel:theEvent];
 }
}

[Neil Hodgson]

Chinh Nguyen:

> I personally don't use the scroll wheel magnification because pressing command-W to close the window while its content is still scrolling due to inertia scrolling will unintentionally change the magnification. For me, the magnification is a setting that's automatically saved so I'll get a really big or really small font size for the next window I open.

Attached is another patch that leaves magnification disabled unless SCROLL_WHEEL_MAGNIFICATION is defined. May be better as a runtime setting but fixing the scrolling is high priority so should be committed soon.

Neil

[Chinh Nguyen]

You might want to add a comment as to why overriding scrollWheel: is necessary. If SCROLL_WHEEL_MAGNIFICATION is not defined, one might wonder what the point of the override is if all it does is call through to super, especially if you're looking at it years later.

[Neil Hodgson]

Chinh Nguyen:

> You might want to add a comment as to why overriding scrollWheel: is necessary.

I don’t really understand why it is necessary. When working on scrolling earlier, implementing scrollWheel turned on responsive scrolling. Here is the discussion from 2013:

https://groups.google.com/d/topic/scintilla-interest/8XF9pl8dTKE/discussion

Neil

[Neil Hodgson]

Committed fix:
https://sourceforge.net/p/scintilla/code/ci/711d6bbf62b6323c3190b67e25cafe79d6320324/

Neil