flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
165.05k stars 27.2k forks source link

Flutter Web icons look pixelated on PixelBook screen (but not external monitor) #63122

Closed eseidelGoogle closed 3 years ago

eseidelGoogle commented 4 years ago

Viewing Rive 2, I see that when I load it on my external monitor (attached to pixel book) it looks fine, but on the built in screen icons look pixelated. @ferhatb asked me to file.

On main screen normal (100%) browser zoom: Screenshot 2020-08-06 at 11 19 47 AM

On external monitor, normal (100%) browser zoom: Screenshot 2020-08-06 at 11 21 02 AM

Main screen at 200% browser zoom: Screenshot 2020-08-06 at 12 52 06 PM

External Monitor 200% browser zoom: Screenshot 2020-08-06 at 12 52 22 PM

I believe Rive2 uses the CanvasKit backend?

eseidelGoogle commented 4 years ago

I believe this is the external monitor: https://www.asus.com/us/Monitors/VG248QE/

yjbanov commented 4 years ago

The pixelation looks similar to one reported in https://github.com/flutter/flutter/issues/62652 for iOS/Safari 14. Wondering if it's the same issue.

@eseidelGoogle Do you see this issue when your laptop is disconnected from the external monitor? (hypothesizing device-pixel ratio confusion when a second monitor is present).

/cc @kjlubick I suspect the fix might be on the Skia side.

eseidelGoogle commented 4 years ago

Here is a screen shot from my pixelbook with no external monitor and the latest build of the app. I've not restarted my pixel book.

image

yjbanov commented 4 years ago

Ok, the issue seems to be in https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/painting/image_resolution.dart. The line https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/painting/image_resolution.dart#L257 causes us to pick assets meant for lower DPR screens, which doesn't work for desktop screens that tend to have DPR <2. For example, my monitor is a quite high-resolution 5K2K display. However, with 1.25 DPR we'd pick 1x image assets and I see unpleasant looking image upscaling artifacts.

A similar issue is https://github.com/flutter/flutter/issues/44108.

I'm going to relabel this as a framework issue. /cc @goderbauer @HansMuller

goderbauer commented 4 years ago

It is not super-clear to me based on what we should pick the correct asset variant if not based on the reported pixel ratio.

yjbanov commented 4 years ago

@goderbauer I agree that we should pick the correct variable based on the device pixel ratio. However, I think the logic we use doesn't work for low DPR screens. I think the following would work better:

  String? _findNearest(SplayTreeMap<double, String> candidates, double value) {
    if (candidates.containsKey(value))
      return candidates[value]!;
    final double? lower = candidates.lastKeyBefore(value);
    final double? upper = candidates.firstKeyAfter(value);
    if (lower == null)
      return candidates[upper];
    if (upper == null)
      return candidates[lower];
    // On low DPR screens, prefer the higher resolution image because using
    // a lower resolution image results in visible scaling artifacts.
    if (value < 2)
      return candidates[upper];
    if (value > (lower + upper) / 2)
      return candidates[upper];
    else
      return candidates[lower];
  }
jonahwilliams commented 4 years ago

Re-posting some comments I made on the discord chat:

What are we actually trying to measure here? For example, if I am trying to draw an image into a square that is 100 physical pixels x 100 physical pixels, then I should choose an image that is sized closest to that, right? if I go under, I'm going to stretch and it will look pixelated and if I go over I'm wasting memory and processor time. but instead of specifying the physical pixels of the image, our asset system specifies the device pixel ratio?

so we are using dpr as a short cut for guessing the image physical pixels without decoding it? but if we knew the exact physical dimensions then we could always pick the best size.

dnfield commented 4 years ago

Also some thoughts from chat from me:

I don't think DPR is a good proxy for this on desktop (including Desktop web).

On mobile, DPR is widely used to constrain logical screen size. On Desktop, it is rarely used that way - desktop idioms assume more logical space is fine, small controls are fine, crowded views are fine, because worst case you have a mouse and keyboard for input and don't need to rely on touch input that will have a hard time picking specific pixels or small controls.

@yjbanov - does this affect mobile web and desktop web?

I do not think we should be rounding up on mobile - the current algorithm seems correct on Android and iOS, and rounding up could result in additional memory consumption for using larger than necessary assets.

jonahwilliams commented 4 years ago

This definitely affects desktop web, the flutter gallery application also looks pixelated on my monitor when served in chrome

dnfield commented 3 years ago

Another thing that affects this: we don't snap to physical pixels.

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.