Subject: Re: bug#63187: 30.0.50; Tail of longer lines painted after end of nearby lines on macOS

From: Aaron Jensen <[email-masked]>
Date: Sun, 25 Jun 2023 15:07:39 -0400

On Sun, Jun 25, 2023 at 2:17 PM Alan Third <[email-masked]> wrote:
>
> On Sun, Jun 25, 2023 at 01:07:19PM -0400, Aaron Jensen wrote:
> >
> > Thank you for the continued explanation. I'm doing some additional
> > digging and it's making me wonder if what we are doing is even
> > possible reliably with Cocoa drawing and without any synchronization
> > primitives like MTLCommandBuffer's waitUntilCompleted. If I understand
> > correctly, we do something like this:
> >
> > 1. Draw to Surface 1 (buffer commands)
> > 2. Assign Surface 1 to the view as its layer
> > 3. Copy from Surface 1 to Surface 2
> > 4. Copy rects on Surface 2 to other parts of Surface 2
>
> Yep.
>
> > As I understand, there's no way to ensure that all commands have been
> > flushed all the way to the point that we know that Surface 1 is
> > actually representative of the commands we have sent it.
>
> No, you can use [NSGraphicsContext flushGraphics] and CGContextFlush
> to force it.

OK, that makes sense, but it's hard to find definitive documentation
of that (and ChatGPT seems to think that it's not true, but that's
likely a hallucination):

=====START GPT=====
[NSGraphicsContext flushGraphics] (in Cocoa) and CGContextFlush (in
Core Graphics) are used to bypass this batching process and force any
pending drawing commands to be executed immediately. After calling
these methods, you can be sure that all your previously issued drawing
commands have been sent to the GPU.

However, it's important to understand that while these functions
ensure that the commands are sent to the GPU, they don't guarantee
that the GPU has finished executing them. The actual rendering work is
still done asynchronously by the GPU.

So if you need to read back the results of your rendering (e.g., from
an IOSurface), there might still be a brief period where the rendering
hasn't completed yet, even after calling flushGraphics or
CGContextFlush. If you need to ensure that all GPU rendering work is
complete before you proceed, you would typically need to use a more
low-level API (like Metal or OpenGL) that provides explicit
synchronization capabilities.

...

Regarding your question about the [NSGraphicsContext flushGraphics]
and CGContextFlush, here are the descriptions directly from Apple's
documentation:

[NSGraphicsContext flushGraphics]: "Forces any buffered drawing
commands to be sent to the destination."

CGContextFlush: "Forces all drawing operations to be completed in the
specified context."

It's important to understand these function calls ensure that drawing
commands are dispatched, not that they are completed. This is an
inference based on understanding of how graphics pipelines generally
work. For more detailed behavior and how these calls interact with
your specific use-case, you should refer to Apple's documentation and
guides, or consider reaching out to Apple's developer support.
=====END GPT=====

> As I recall we've tried that in both copyRect and display.
>
> Ultimately releasing the context will have to flush any buffered
> writes, if it didn't then this would be a widely known problem as
> practically nothing graphical could be relied upon.
>
> This is my point about why I can't see this being the issue. Although
> it looks like it explains what we're seeing, we would have to be
> hitting some extreme edge-case.
>
> > I came across some notes from the Chromium team:
> > https://www.chromium.org/developers/design-documents/iosurface-meeting-notes/
> >
> > They (as of the notes) decided not to double buffer and just write
> > directly to the one surface. I don't know if that's reasonable/viable
> > for Emacs or not.
>
> Set CACHE_MAX_SIZE to 1.
>
> But on my machine this resulted in unacceptable rendering flaws on
> almost all animated gifs as partially drawn buffers were sent to VRAM.

What did you see?
I tried with this:

https://share.cleanshot.com/vJTClHW9

And I didn't notice anything drawing related, but I have an M1 with
integrated memory.

Aaron

> And adding more open frames just made it worse, as I recall. I never
> really understood why.
>
> It wasn't just gifs, but they were an easy test.
>
> But then, I'll bet performance is incredible. ;)
>
> > Lastly, as an aside, if we *could* synchronize, would it be more
> > efficient to not copy the entire surface just to then copy rects again
> > to handle scrolling? I can imagine this would add more complexity than
> > is worth it, I'm mainly asking for my own edification.
>
> I'm not sure if I'm following your question correctly, but yes, not
> copying is usually going to be faster, but it depends how long it
> takes the buffer to copy to VRAM, you might well find that you're
> waiting for it to finish when you may have already been in the
> position to start drawing to a second buffer.
>
> In theory, if you set CACHE_MAX_SIZE > 1, and change
>
>     [cache addObject:(id)currentSurface];
>
> to
>
>     [cache insertObject:(id)currentSurface
>                 atIndex:0]
>
> It should try to pick up the most recent surface first, then
> copyContentsTo will recognise that it's the same surface as last time,
> and not bother with the copy. I think you'd have to enable
>
>     [self setContents:nil];
>
> to work all the time too, as you might be wanting to "replace" the
> contents layer with itself.
>
> I've no idea if that would really work. IIRC on my machine the surface
> that was "in flight" to VRAM was never done fast enough that it would
> be picked up by the next getContext call so I felt it was a waste to
> check it every time. (And in fast-scrolling situations I could have as
> many as three surfaces in-flight at once if I let the cache grow that
> big.)
> --
> Alan Third