Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add anti aliasing support for thumbnails #705

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

owwlo
Copy link

@owwlo owwlo commented Nov 22, 2020

With it being OFF:
Screen Shot 2020-11-21 at 4 29 26 PM

ON"
Screen Shot 2020-11-21 at 4 29 40 PM

@owwlo
Copy link
Author

owwlo commented Nov 22, 2020

@lwouis, first, thanks for creating this awesome project. I have been a heavy alt-tab-macos user ever since. This patch brings in the capability of displaying anti-aliasing scaled thumbnails(as you can see from the screenshots). This is literally my first ever xcode/swift code change. I hope I didn't mess things up by... too much ;).

@lwouis
Copy link
Owner

lwouis commented Nov 23, 2020

Thank you for sharing this PR! I'm interested to look into it more. I have played with interpolation previously. In the past I ran into performance issues, especially when the screens are 4k, and thus the thumbnails we get from the system are multiple megabytes each.

This means I will have to test the performance of your change, which is not an easy thing to do, as well as study the qualitative image improvements involved. Some interpolation algos have downsides in certain scenarios.

I'll look into this as soon as I have some free time. Again, thank you very much for sharing your work! 👍

@lwouis
Copy link
Owner

lwouis commented Dec 21, 2020

Hi @owwlo! So I could finally test your work.

The visual results are, I would agree, an improvement. Not a huge one, but still it's easier to read text on the thumbnails with your change.

Now when I tested the performance, I noticed, as I expected, a slowdown (left is before, right is after):

image

You can see on the right how the blue bar are wider generally, meaning more time needed to display AltTab's UI. And you can see at the bottom that on a few invocations with the shortcut, the image scaling alone took half a second. This is extra work that you added to process the image, and that is a net negative on the responsiveness.

The thing is that NSImageView is already doing its own interpolation internally. When that view is resized, it will downsample the NSImage it's hosting. So we are basically doing another pass on top of that, which is a bit wasteful.

In this article, the person is subclassing NSImageView and disabling interpolation. Maybe you could use the same technique to set it to medium, as you have done in this PR. Also, I'm curious about what value it is using currently.

Finally, how is high looking like? How about low? If these have a performance/quality tradeoff, we could imagine letting users pick through a dropdown menu, so they can make the tradeoff they prefer.

@owwlo
Copy link
Author

owwlo commented Dec 24, 2020

Hey @lwouis,

Thanks a lot for the results and... wow! It indeed slows down the process by a lot! I didn't expect it could cost that much(1016ms vs 2124ms). I will learn to profile the code in my next PR.

In this article, the person is subclassing NSImageView and disabling interpolation. Maybe you could use the same technique to set it to medium, as you have done in this PR.

I will try that!

I'm curious about what value it is using currently.

I highly suspect it's low cause when I work on this feature, I tried all the settings. medium and high give pretty much the same results(at least cannot distinguish with my naked eyes), while low looks the same as it is shown in NSImageView.

If these have a performance/quality tradeoff, we could imagine letting users pick through a dropdown menu, so they can make the tradeoff they prefer.

Good point! I will add a dropdown menu.

@owwlo
Copy link
Author

owwlo commented Dec 28, 2020

Hey @lwouis, some updates:

[About overriding NSImageView::draw to set the Interpolation level]

I did some experiments to override NSImageView::draw and set NSGraphicsContext.current.imageInterpolation tonone/high. But sadly, I didn't see any difference during runtime... which led me to suspect that there might be no interpolation involving by default. To prove that, I did some profiling:

image
(In this experiment, I set NSGraphicsContext.current!.imageInterpolation = NSImageInterpolation.high. It turns out draw takes only 1ms. After comparing it with the other numbers in my experiments, I believe, in our case, NSImageView does not apply any scaling process to the image)

[Make anti aliasing a dropdown option]

The dropdown is added with options: None, Low, Medium and High.

  • By default, None is chosen
  • If None is selected, no scaling will apply(updateThumbnailIfNeeded take 1ms - no performance impact)
  • I realized you have had the exact function I needed in HelperExtensions.swift. I modify it slightly to reuse the code

Some profiling:
image
(None(from above) vs. Low vs. Medium: 1ms/87ms/118ms)

======

I agree this is not a huge improvement in any way. But as an AltTab heavy user(thanks again for founding such an awesome project!), after a month of trying this feature, it indeed brings some lightness every time I press alt-tab. I do realize that the performance penalties in some cases could be heavy... so if you would like to put this feature on hold, I would totally understand.

Let me know what you think.

@lwouis
Copy link
Owner

lwouis commented Jan 26, 2021

Hey @owwlo, sorry for the late response!

I would like to dig deeper into the topic before moving forward to integrating this enhancement. I'd like to learn more about NSImageView inner workings:

  • Does it really not do any interpolation? The post I shared above seems to demonstrate that it does
  • How could we do interpolation within that class?

I'm thinking that modern computers are really fast at graphics scaling, especially when leveraging the GPU properly, and that there must be ways to make the images better with almost no performance cost.

@lwouis
Copy link
Owner

lwouis commented Mar 8, 2021

I've worked on this topic for a few hours today.

I tried the subclassing method:

class InterpolatedImageView: NSImageView {
    override func draw(_ dirtyRect: NSRect) {
        NSGraphicsContext.current!.imageInterpolation = NSImageInterpolation.high
        super.draw(dirtyRect)
    }
}

I also tried this CGContext method:

extension NSImage {
    func resized(size: NSSize) -> NSImage {
        let intSize = NSSize(width: Int(size.width), height: Int(size.height))
        let cgImage = self.cgImage(forProposedRect: nil, context: NSGraphicsContext.current, hints: nil)!
        let bitsPerComponent = cgImage.bitsPerComponent
        let colorSpace = cgImage.colorSpace!
        let bitmapInfo = CGImageAlphaInfo.noneSkipLast
        let context = CGContext(data: nil,
            width: Int(intSize.width),
            height: Int(intSize.height),
            bitsPerComponent: bitsPerComponent,
            bytesPerRow: 0,
            space: colorSpace,
            bitmapInfo: bitmapInfo.rawValue)!

        context.interpolationQuality = .high
        context.draw(cgImage,
            in: NSRect(x: 0, y: 0, width: intSize.width, height: intSize.height))
        let img = context.makeImage()!
        return NSImage(cgImage: img, size: intSize)
    }
}

First of all, here is a study on visual quality. You can see how 2 thumbnails look on different settings:

Current Sub-class, no interpolation Sub-class, low interpolation Sub-class, medium interpolation Sub-class, high interpolation Sub-class, default interpolation Sub-class, high interpolation + shouldAntialias CGContext, high-interpolation
current default high-antialias high low medium none cgcontext-resize

The current quality is interesting because it doesn't match any other result. It seems to be between low and medium. It's probably handled differently.

Regarding performance, both approaches have a penalty. It's not huge (on my powerful machine), but it's significant. You can see the cost on these profiles:

Current Sub-class, high interpolation CGContext, high-interpolation
image image image

I stay convinced that there is a way to do this resizing without cost. The 2 approaches above must involve a memory copy, or be done on the CPU instead of the GPU, or something of that nature. Resizing an image should be instant on a GPU. As proof, it is currently instant when done within stock NSImageView.

We could just stick this in the project, mitigated with a dropdown in the preferences so users can lower interpolation if they want a snappier experience. However, I would like for us to dig deeper, and try to find why we have overhead.

Here's where the time is spent:

Sub-class, high interpolation CGContext, high-interpolation
image image

Looks to me like we need to avoid this img_interpolate_read

@owwlo
Copy link
Author

owwlo commented Mar 15, 2021

@lwouis, sooooo sorry for the late reply. I got tied up with work recently.

2 weeks ago, I was playing with it during the weekend but didn't get much progress.

class InterpolatedImageView: NSImageView {
    override func draw(_ dirtyRect: NSRect) {
        NSGraphicsContext.current!.imageInterpolation = NSImageInterpolation.high
        super.draw(dirtyRect)
    }
}

Interestingly, I tried the exact same snippet. I tried imageInterpolation={none, low, high} but never get any improvement over the existing thumbnails. Could you share your code? I doubt that I missed or did something wrong with my implementation.

I stay convinced that there is a way to do this resizing without cost... Resizing an image should be instant on a GPU. As proof, it is currently instant when done within stock NSImageView.
...
However, I would like for us to dig deeper, and try to find why we have overhead.

I would totally agree, and I will definitely keep digging and will keep you posted in this thread(hopefully, will clear all the stuff on my hand by the end of this week 🤞 🤞 ).

@lwouis
Copy link
Owner

lwouis commented Mar 15, 2021

@owwlo

Could you share your code?

I did above. It's pretty much it. The only missing thing I guess is replacing the type with InterpolatedImageView here.

For the CGContext method, I added the processing code somewhere, I forgot. It was a one off to test quality, so I didn't do a full proper implementation that handles resizing correctly in all the cases.

I would totally agree, and I will definitely keep digging and will keep you posted in this thread(hopefully, will clear all the stuff on my hand by the end of this week 🤞 🤞 ).

I know how it is! Some days I feel overwhelmed with this project, as people keep pilling on the tickets, and very few people contribute. So take your time, and hopefully we improve image quality eventually 👍

@owwlo
Copy link
Author

owwlo commented Mar 29, 2021

@lwouis, just to share some interesting findings:

Here is my codebase setup:

  • To avoid potential result differences from mismatch revisions, I cloned a fresh master repo today for the test
  • Add a new InterpolatedImageView:
class InterpolatedImageView: NSImageView {
    override func draw(_ dirtyRect: NSRect) {
        NSGraphicsContext.current!.imageInterpolation = NSImageInterpolation.{none, high}
        super.draw(dirtyRect)
    }
}
  • Use it for var thumbnail:
class ThumbnailView: NSStackView {
    var window_: Window?
    var thumbnail = InterpolatedImageView()
    var appIcon = NSImageView()
......

I tweak NSGraphicsContext.current!.imageInterpolation = NSImageInterpolation.{none, high} and here is what I got:
[.high]
NSImageInterpolation high
[.none]
NSImageInterpolation none

As you can see, they both produce identical results. @lwouis, do you see anything I missed in my experiments :) ?

I even thought about the Retina Resolution(I used non-retina display to capture the results above). I tried the same code on my 13' rMBP built-in display and barely see the difference either.

@lwouis
Copy link
Owner

lwouis commented Mar 31, 2021

@owwlo that's weird. Here is a branch where I added the changes. It's the latest commit on that branch. It's basically what I wrote above, that you say you did without success. With that code change, I see the impact. When I replace .high with .none I see a dramatic drop in quality for example.

@owwlo
Copy link
Author

owwlo commented Apr 18, 2021

@lwouis, just curious, do you use Big Sur? I searched online around and found https://developer.apple.com/forums/thread/658797?login=true&page=1#671189022.

@lwouis
Copy link
Owner

lwouis commented Apr 19, 2021

@owwlo oh that may explain our difference in behavior. I'm on macOS Catalina (10.15.7).

@lwouis
Copy link
Owner

lwouis commented May 22, 2022

@owwlo I was decompiling HyperSwitch today, and found how they render some thumbnails. Here's some code they call:

CGContextSetShouldAntialias(rax, 0x0);
CGContextSetAllowsAntialiasing(r15, 0x0);
CGContextSetInterpolationQuality(r15, r12);

That being said, they don't do on-the-fly screenshots, but use the slower public APIs like CGWindowListCreateImageFromArray, then create/refresh the thumbnails async. So not a model to imitate for AltTab, but interesting to see how they handle quality with antialiasing and interpolationQuality on a CGContext.

This reminded me of this PR. It's kind of a shame we never finished it. Do you think you could look at it again?

@jacksongoode
Copy link

👀 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants