#2292 Margin does not redraw when scrolling slowly upwards on macOS 12 Monterey

[Rowan Daniell]

It appears that Monterey does not like synchronous redraw.
This is easily reproducible in the ScintillaTest app running on Monterey. Scroll down, then scroll up slowly. The revealed margin area is not drawn. See screen shot.

[Neil Hodgson]

OK. I don't yet have a Mac that can run Monterey so can't reproduce the problem. Is the backwards scrolling still sticking to whole lines?

The image isn't showing syntax highlighting so there could be something else failing to cause this. Lexilla needs to be installed into a "lexilla" directory that is a peer of "scintilla" for the build to work and perform syntax highlighting.

[Rowan Daniell]

Yes, scrolling upwards sticks to whole lines. This redraw issue on Monterey was present with Scintilla 4.4.4. I updated to 5.1.3 this morning and the behaviour is the same. It happens with or without syntax highlighting.

I have been playing around and I think I have bodged a fix by changing the end of ScintillaCocoa::SyncPaint() to this

if (!succeeded || true) { // always
    NSView *marginView = (__bridge NSView *)(wMargin.GetID());
    [marginView setNeedsDisplay: YES];
    return false; // force caller to setNeedsDisplay:YES
}
return succeeded; //notreached

}

i.e. always refresh margin and force caller to always setNeedsDisplay
I'm sure there must be a less heavy-handed way. Happy to test if you have a suggestion.
It is of course always possible that the next update of Monterey will change the behaviour.

[Neil Hodgson]

The return false causes an extra full redraw after every draw, The picture only shows the margin drawing incorrectly, so a lighter change would only redraw the margin, returning succeeded.

As this problem has only been reported on Monterey, this extra cost can also be avoided on earlier versions of macOS.

if (@available(macOS 12.0, *)) {
        NSView *marginView = (__bridge NSView *)(wMargin.GetID());
        [marginView setNeedsDisplay: YES];
} else if (!succeeded) {
        NSView *marginView = (__bridge NSView *)(wMargin.GetID());
        [marginView setNeedsDisplay: YES];
}
return succeeded;

[Rowan Daniell]

Sorry Neil, that doesn't work. If you don't return false on Monterey, the margin does not draw. I'm not entirely sure why that is (but then the dispatch block in drawRect: calling setneedsDisplay: is kind of unusual).

[Neil Hodgson]

If you don't return false on Monterey, the margin does not draw.

That doesn't make any sense to me.

I still think its worth avoiding the extra redraw costs on earlier macOS so the new case could go in an @available(macOS 12.0 leaving the current code for older systems.

if (@available(macOS 12.0, *)) {
        NSView *marginView = (__bridge NSView *)(wMargin.GetID());
        [marginView setNeedsDisplay: YES];
        return false; // force caller to setNeedsDisplay:YES
}
if (!succeeded) {
        NSView *marginView = (__bridge NSView *)(wMargin.GetID());
        [marginView setNeedsDisplay: YES];
}
return succeeded;

[Rowan Daniell]

Unfortunately (and I suppose not surprisingly), the always false return sets up an endless redraw cycle (not just one extra draw). Not really noticeable until you look at the CPU usage.

[Neil Hodgson]

sets up an endless redraw cycle

Yes, that's what I expected.

(but then the dispatch block in drawRect: calling setneedsDisplay: is kind of unusual)

When you return from drawRect, the needs-display area is cleared so performing setNeedsDisplay before returning has no effect. Delaying the setNeedsDisplay to the next run-loop iteration ensures it really occurs. A similar delayed setNeedsDisplay could be performed on the margin view if you can determine a good place to do that.

[Rowan Daniell]

I think the problem arises from the content view trying to paint the margin view from its drawRect:

Solution would be to paint the margin from the margin view's drawRect:

I added this at ScintillaView.mm:205 (ie in SCIMarginView)

- (void) drawRect: (NSRect) rect {
    owner.backend->PaintMargin(rect);
}

and removed the previous hack in SyncPaint. Seems to work (only tested with a basic margin, Catalina and Monterey)

[Neil Hodgson]

The SCIMarginView should normally be painted from its drawHashMarksAndLabelsInRect which is called in a superclass's drawRect.

Can you check whether drawHashMarksAndLabelsInRect is called on macOS 12 before adding the drawRect implementation?

The drawRect addition has been committed with [e6a116] but, for efficiency, I'd like to find out what is happening and remove any duplicated drawing code.

Also watch for drawing artifacts from the corners when scrolling as there is some compensating code in drawHashMarksAndLabelsInRect. SciTE added a grey bottom bar to avoid some issues with rounded bottom window corners.

[Chinh Nguyen]

FWIW, adding:

- (void) drawRect: (NSRect) rect {
    [super drawRect:rect] ;
}

instead to SCIMarginView solves the problem.

[Rowan Daniell]

So it does! That seems weird.

edit: That looks like the cleanest solution

[Neil Hodgson]

Committed this as [787324] since it should be less of a change.

[Rowan Daniell]

It is called, but not consistently. The occasions where the margin is not drawn correspond to drawHashMarksAndLabelsInRect : not being called. And of course it never gets called at all when drawRect: is overridden (didn't know that NSRulerView has that default behaviour for drawRect:). This would imply that the default drawRect: for NSRulerView is deciding not to call drawHashMarksAndLabelsInRect : sometimes (i.e. for a small upward scroll).

So now I have this

- (void) drawRect: (NSRect) rect {
    [super drawRect: rect];
    if(owner)
        owner.backend->PaintMargin(rect);
}

but it will double draw most of the time (could add a didDraw flag to check what the superclass did...). Source code for NSRulerView would be useful.

[Neil Hodgson]

The occasions where the margin is not drawn correspond to drawHashMarksAndLabelsInRect : not being called.

I wonder if this corresponds to times where a scroll of less than a full line is quantized up or down to a full line by adjustScroll.

[Chinh Nguyen]

I can confirm that bug still exists in the 12.1 Beta (21C5039b) of Monterey which is the latest beta as of now.

From my cursory testing I can confirm that the fix works.

[Vladislav Peichev]

Still reproducible with Scintilla 5.1.5 on Mac OS Monterey 12.0.1.

[Vladislav Peichev]

Sorry, my mistake - my application was using outdated old framework files. The issue is fixed.