Add macOS support using Core Graphics

[sanxiyn]

It draws something, but I am not confident it is correct. I am especially worried about coordinate system.

What should I expect to see for winit example? I think it would help to include screenshots in README (plural, to show resizing), and for example to be recognizably different when horizontally or vertically flipped. I propose a 45-60-75 triangle.

[john01dav]

I've added a new fruit example to show a proper image that I got off Wikimedia Commons. That should test a bit more thoroughly than a simple triangle since it has lots of colors and is otherwise more complicated so more can go wrongly. I'll probably add something to the README later, but I wanted to get this to you quickly.

Here are some screenshots (2 on Fedora Linux and two on Windows 10, note that when the window is larger than the displayed image it doesn't matter what is displayed and is different between Linux and Windows):

[john01dav]

Here is the winit example on both Linux and Windows. Note which side of the squares each color shows up on:

[sanxiyn]

Thanks! That was helpful. I am now confident this works correctly.

[john01dav]

Thanks for the PR!

[john01dav]

@sanxiyn I'm updating the README to reflect the new AppKit support. Do you have a preference for what name you are credited with?

[sanxiyn]

Please use my Git user.name, that is Seo Sanghyeon.

[john01dav]

@sanxiyn I recently got access to a Mac computer of my own (albeit an old one, it supports nothing after High Sierra) and I was testing some related code on it and I found some cases in your implementation that don't work correctly. I was able to solve one of them (drawing an empty image causes an assertion to fail, so it's easy enough to fix by simply not drawing the image if width and height are zero), but I was unable to fix the other.

It seems that once an image is drawn to the window, it is impossible to draw to it again with a new image until after the window resizes. I tried to fix this, but my unfamiliarity with the core graphics API made this not work out. I'll keep trying, but you might have a better chance to figure it out.

I've added a new animation example that reliably reproduces the bug on Mac OS. Here's a video of what it is supposed to look like. I had to put it in a zip so Github would accept the upload. This example also lags pretty hard on Mac OS (but not on Windows or Linux, except when resizing as it blocks the main thread to re-render the animation, but even if that is an issue it's a winit one or an issue with the example and not a softbuffer one -- it lags on Mac OS when set_buffer alone is spammed).

[kpreid]

It seems that once an image is drawn to the window, it is impossible to draw to it again with a new image until after the window resizes.

I'm seeing the same behavior you describe on macOS Monterey 12.2 (with cargo run --example animation --release, or with my own code).

Unfortunately I also don't have any ideas about how to fix it, or at least not ones that worked. I was wondering if it was missing some sort of “flush” operation, but msg_send![graphics_context, flushGraphics] and context.flush(); changed nothing. Accessing/creating the graphics context anew for each set_buffer didn't work entirely. Another idea I had was that maybe the graphics context wasn't available as intended (perhaps set to clip everything) except when resizing, but if that's so, I don't see what to change to fix that, short of implementing a “proper” NSView subclass. (I used to work with AppKit years ago, but never Core Graphics.)

[john01dav]

Something else that might be relevant is this ( rust-windowing/winit#1605), but unfortunately my attempt to use it didn't work (described in the linked issue). Maybe this means more to you? If that doesn't work I'll probably have to contact Apple (and who knows if they'll be helpful), or give up and have a GPU dependent buffer on Mac OS, as minifb does.

…

On Mon, Feb 7, 2022, 12:06 AM Kevin Reid ***@***.***> wrote: It seems that once an image is drawn to the window, it is impossible to draw to it again with a new image until after the window resizes. I'm seeing the same behavior you describe on macOS Monterey 12.2 (with cargo run --example animation --release, or with my own code). Unfortunately I also don't have any ideas about how to fix it, or at least not ones that worked. I was wondering if it was missing some sort of “flush” operation, but msg_send![graphics_context, flushGraphics] and context.flush(); changed nothing. Accessing/creating the graphics context anew for each set_buffer didn't work entirely. Another idea I had was that maybe the graphics context wasn't available as intended (perhaps set to clip everything) except when resizing, but if that's so, I don't see what to change to fix that, short of implementing a “proper” NSView subclass. (I used to work with AppKit years ago, but never Core Graphics.) — Reply to this email directly, view it on GitHub <#1 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA6MJIORAM5E7LOFJLL3NHTUZ5ONJANCNFSM5MIY3TSA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[kpreid]

I'm not quite following. You say it doesn't work here, but when I follow the link I see something that says it would work if winit was changed to call setNeedsDisplay? (Which sounds like a fine idea to me — except that it might be an extra cost to applications that don't need it, and it means that softbuffer would need to document that drawing must be done from RedrawRequested, rather than that it can be done there.)

Another likely correctness issue here is that the current set_buffer() code uses the thread current graphics context rather than one obtained from the window and remembered or re-fetched; this would surely draw in the wrong place if there are multiple windows, and I suspect the current behavior has to do with the state of that context. However, I tried to fix that and both of the approaches I tried resulted in no drawing at all rather than the current sometimes-drawing.

Maybe the GPU dependent drawing is the better answer all around, since it means you get a layer of your own independent of what the AppKit window drawing is doing.

[john01dav]

It turned out that I had missed something obvious in my testing of setNeedsDisplay. It was actually working, but because I had slowed the update rate to 1Hz and because the animation is one second long it was showing the same frame each time and thus appearing to not work. Once I increased the animation speed back to where it should be, everything works. I have now made a PR to winit about setNeedsDisplay.

[tinaun]

I do want to try an alternative pr using a custom layer backed NSView at some point

[john01dav]

@tinaun I always want the code to be as correct as possible, but I am unsure what the advantage is of doing it that way. Is it just a general sense of correctness (and there's nothing wrong with this), or is there some more practical advantage? Please don't take this to be discouraging in any way. I'm just trying to understand your motivation and any potential shortcomings of the current solution.

[tinaun]

• this will (hopefully) support scaling in retina modes better, as the current examples always draw the images in logical pixels not physical pixels
• your pr for using setNeedsDisplay in winit breaks opengl/metal
• that was the approach I was going to try before knowing this repo existed :v

[kpreid]

as the current examples always draw the images in logical pixels not physical pixels

In some applications this might be a desirable option, as the application might be performing an expensive per-pixel operation (e.g. raytracing, fractals), and retina/hidipi displays can be a surprising additional cost, especially if the developer hasn't tested on one themselves. Of course, programmable scale / resampling is even better, but that probably gets into GPU-drawing territory.

But, in any case, softbuffer needs a consistent interpretation of sizes — the documentation of GraphicsContext::set_buffer says “Use your windowing library to find the size of the window.” and there must be a correct answer to which size which does not depend on platform. The top-level doc example uses winit's Window::inner_size, which is a PhysicalSize, which argues for indeed using the physical pixels.

[john01dav]

@tinaun Those are great reasons, thank you for clarifying. If you make the PR I'll definitely merge it.