Skip to content

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

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

Closed
eseidelGoogle opened this issue Aug 6, 2020 · 11 comments · Fixed by #69799
Closed
Assignees
Labels
a: desktop Running on desktop a: images Loading, displaying, rendering images customer: thrive customer: web10 e: device-specific Only manifests on certain devices e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web framework flutter/packages/flutter repository. See also f: labels. P1 High-priority issues at the top of the work list platform-web Web applications specifically

Comments

@eseidelGoogle
Copy link
Contributor

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
Copy link
Contributor Author

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

@TahaTesser TahaTesser added e: device-specific Only manifests on certain devices e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web engine flutter/engine repository. See also e: labels. passed first triage platform-web Web applications specifically labels Aug 7, 2020
@yjbanov yjbanov added P1 High-priority issues at the top of the work list customer: web10 labels Aug 10, 2020
@yjbanov
Copy link
Contributor

yjbanov commented Aug 10, 2020

The pixelation looks similar to one reported in #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
Copy link
Contributor Author

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
Copy link
Contributor

yjbanov commented Aug 11, 2020

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 #44108.

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

@yjbanov yjbanov added framework flutter/packages/flutter repository. See also f: labels. and removed e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web engine flutter/engine repository. See also e: labels. platform-web Web applications specifically labels Aug 11, 2020
@goderbauer goderbauer added a: desktop Running on desktop a: images Loading, displaying, rendering images labels Aug 17, 2020
@goderbauer
Copy link
Member

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
Copy link
Contributor

yjbanov commented Aug 29, 2020

@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];
  }

@yjbanov yjbanov added platform-web Web applications specifically e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web and removed a: desktop Running on desktop labels Sep 16, 2020
@jonahwilliams
Copy link
Member

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.

@goderbauer goderbauer added the a: desktop Running on desktop label Sep 21, 2020
@dnfield
Copy link
Contributor

dnfield commented Sep 21, 2020

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
Copy link
Member

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

@dnfield
Copy link
Contributor

dnfield commented Nov 2, 2020

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

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: desktop Running on desktop a: images Loading, displaying, rendering images customer: thrive customer: web10 e: device-specific Only manifests on certain devices e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web framework flutter/packages/flutter repository. See also f: labels. P1 High-priority issues at the top of the work list platform-web Web applications specifically
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants