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

UIImageKnob, store 2 downsampled source image in KnobImage #791

Open
p0nce opened this issue Sep 6, 2023 · 3 comments
Open

UIImageKnob, store 2 downsampled source image in KnobImage #791

p0nce opened this issue Sep 6, 2023 · 3 comments
Labels
Enhancement This issue is about a new feature rather than a bug. Performance This issue is about performance enhancement.

Comments

@p0nce
Copy link
Collaborator

p0nce commented Sep 6, 2023

Perhaps the resizing algo is now a bit too aggro for Diffuse map. Best see in macOS, Auburn Sounds latest thing, PBR diffuse has slight halos. It has a blurry and "detached" look.

(EDIT: actually very annoying)

@p0nce p0nce added the Enhancement This issue is about a new feature rather than a bug. label Sep 6, 2023
@p0nce p0nce changed the title Diffuse resize a big aggressive. Diffuse resize a bit aggressive. Sep 16, 2023
@p0nce
Copy link
Collaborator Author

p0nce commented Oct 12, 2023

I think this is instead due to UIImageKnob resampling twice.
Slight blurry feel on Inner Pitch, the big knob on rotated knob, since it's resampling once for storing the image, and another one for displaying it. Wasteful!

@p0nce p0nce changed the title Diffuse resize a bit aggressive. UIImageKnob new format should resample once Oct 12, 2023
@p0nce p0nce added Performance This issue is about performance enhancement. Low interest Importance of this issue seems questionable for the bottom-line.. labels Oct 12, 2023
@p0nce p0nce removed the Low interest Importance of this issue seems questionable for the bottom-line.. label Nov 22, 2023
@p0nce
Copy link
Collaborator Author

p0nce commented Nov 29, 2023

  • Is the issue the fact we sample twice or the fact we sample the second time without consideration for being alpha-premultiplied? => but interpolation already works correctly for premultiplied, instead it is incorrect for unassociated alpha.

  • Visual test. Linear interp on original image doesn't win particularly against a resized, cubic-interpolated, oversampled, temp image. In particular the problematic halos are still there, not really understood.

  • New method is shifted of (-0.5, -0.5), what is the correct sampling point? A separate issue.

  • What if we also use cubic interpolation? =>effectively sharper, need to see on use vs baseline. Still doesn't solve white halo, and smaller size are aliased.

    • direct sampling noticeably faster on resize and first opening, reflow does nothing there.
    • memory usage? 14% memory gain on Inner Pitch (10.5mb saved per instance)
    • does it beat original-without-oversampling at normal sizes? Interesting. The oversampled original has that "remote" look and feels detached from the UI, a bit blurrier. The new method has a bit more aliasing artefacts but feels more real.
  • test on larger screen => method would need x2 or x4 reduced images to be really cool. mipmap reduced would be based upon scale factor.

  • Alternatively we could store downsized version of the source image in KnobImage, that makes one mipmap precompute instead of resizing UIKnobImages all the time. But: still a case of double-interpolation for smaller sizes, which seems OK.

@p0nce
Copy link
Collaborator Author

p0nce commented Nov 29, 2023

In A/B unclear who wins between 1.3 oversampled baseline, vs direct cubic sampling. and for smaller size it's worse so we need resampling for smaller sizes.

Proper fix for that is:

  • to find real reason for white halo => White halos #825 (very old issue actually)
  • Put a Mipmap (or collections of 3 images) of high-quality (better than we have) in KnobImage, so that resize and multiple widgets don't store their own version. Probably just 2 downsampled levels are sufficient in addition to level 0. That wins the additional CPU on resize, the memory per-widget, with a bit of additional quality when size is not far from original (but, upsampled might look a tiny more pixelated, but assets should be done at max res)

@p0nce p0nce changed the title UIImageKnob new format should resample once UIImageKnob, store 2 downsampled source image in KnobImage Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement This issue is about a new feature rather than a bug. Performance This issue is about performance enhancement.
Projects
None yet
Development

No branches or pull requests

1 participant