mattermost / desktop

Mattermost Desktop application for Windows, Mac and Linux
Apache License 2.0
2.02k stars 829 forks source link

Windows native client has bad anti-aliasing #560

Closed fadeevab closed 2 years ago

fadeevab commented 7 years ago

Steps to reproduce

Open Windows native client and see

Expected behavior

Fonts have a good quality

Observed behavior (that appears unintentional)

firefox_vs_native Left: Firefox has good anti-aliasing. Right: Windows native client has bad anti-aliasing at the same system. Font: Open Sans.

Seems like there are different anti-aliasing algorithms are enforced. At least. Left is Firefox. Right is Mattermost Windows Native client.

firefox_vs_native2

Possible fixes

CSS anti-aliasing settings?

yuya-oc commented 7 years ago

What version are you using? We know v3.6.0 had a rendering issue, and believe that it's improved in v3.7.0.

If you still feel it's not good, would you compare to Chrome? The rendering engine is same. So if they have different result, we might be able to consider something.

fadeevab commented 7 years ago

I'm using v.3.7.0.

P.S.: Interesting thing, that Account Settings -> Display -> Display Font list itself looks better that the applied fonts in the chat. So I zoomed in and what I see:

fonts_list

Above: fonts list field before rolling out. Below: the rolled out list with fonts items. There are different types of anti-aliasing algorithms.

As far as I know that algorithms could be chosen in CSS.

fadeevab commented 7 years ago

Chrome looks better, not so blur.

Left: Chrome Right: Windows native app (greyscale anti-aliasing?)

chrome_vs_native

Result of comparison is the same as in above comments.

yuya-oc commented 7 years ago

Confirmed on Windows 10 with 100% of DPI scaling. But it looks fine for me in other DPIs like 125 (actually they have different rendering).

Left: Chrome, Right: Mattermost Desktop 125

It happens when using a traditional lower DPI and looks Electron's limitation. https://github.com/electron/electron/issues/8708

fadeevab commented 7 years ago

@yuya-oc, Mybe it's better than in previous version, but I have 125% DPI scaling by default, and I find it blurry as well as for 100% DPI, especially when you watch it right after browser version.

FYI, Visual Studio Code is on Electron and it has a good anti-aliasing with algorithm like in my examples at the left side. There are lot of streams on github about their struggling with anti-aliasing: maybe some information would be helpful for you.

fadeevab commented 7 years ago

https://github.com/electron/electron/issues/8708 is solved, whether the latest version of Electron is used in your desktop client? (edited) P.S.: I mean the issue when there two monitors with different DPI... Still, I have two monitors but with the same DPIs.....

fadeevab commented 7 years ago

P.S.: When I enable display mode to show only one display, restart Mattermost, but it's still grayscale anti-aliased in Windows.

yuya-oc commented 7 years ago

@fadeevab The next version, v3.8.0 uses newer Electron that supports different DPI. However it would not resolve the original problem because their reasons are different.

For reference, would you tell me the issue you saw of VSCode?

fadeevab commented 7 years ago

Let me check wait a minute

fadeevab commented 7 years ago

The nuance is that VS Code doesn't have an issue with anti-aliasing at the same Windows at the same time. vscode

jasonblais commented 7 years ago

@yuya-oc Does this still appear to be an issue with Electron?

yuya-oc commented 7 years ago

@jasonblais I couldn't find other open issues. But as far as looking the conclusion of electron/electron#8708, I felt they think it's not fixable.

fadeevab commented 7 years ago

@yuya-oc, How do you explain, that VSCode has proper subpixel anti-aliasing, not grayscale (my latest screenshot)?

yuya-oc commented 7 years ago

@fadeevab I'm worried because I'm not sure why the difference appears. As you said "struggling", it seems not easy.

I tried adding CSS like https://github.com/Microsoft/vscode/issues/1896#issuecomment-315463143 by using DevTools console. But I couldn't get better anti-aliasing.

* {
  -webkit-font-smoothing: antialiased !important;
  text-rendering: optimizeLegibility !important;
}

Left: Chrome, Right: Mattermost Desktop css-antialias

The link below is a minimum implementation of mattermost desktop which uses the latest Electron 1.7.4 (the master branch uses 1.6.10) and CSS insertion like above.

https://www.dropbox.com/s/jcan9895e5bqap1/mattermost-minimum-win32-ia32.zip?dl=0

Wondering if you could test this application and try modifying CSS by using DevTools(Ctrl+Shift+I).

fadeevab commented 7 years ago

(edited): I tested at the one display laptop with 100% DPI, Windows 7.

I did everything and more with CSS, nothing help.

I downloaded and played with basic Electron Demo CSS by toggling developer tools, and I can't make to force subpixel-antialiasing. Electron Demo has grayscale anti-aliasing from the scratch (at least in Windows).

If to download chromium and open index.html with <body>test</body> there is a subpixel antialiasing.

Seems like the Electron issue. So I suppose VSCode team fought with Electron and found a workaround.

More info about why anti-aliasing could be broken: https://github.com/electron/electron/issues/6344#issuecomment-230773130.

Anti-aliasing could be turned into grayscale by a couple of reasons. One of them is a 3d transform. I just saw in VSCode lines like:

    if (canUseTranslate3d() && !isWindows /* Windows: translate3d breaks subpixel-antialias (ClearType) unless a background is defined */) {
    const transform = `translate3d(0px, -${renderTop}px, 0px)`;
    this.rowsContainer.style.transform = transform;
    this.rowsContainer.style.webkitTransform = transform;
    } else {
        this.rowsContainer.style.top = `-${renderTop}px`;
    }

See https://github.com/Microsoft/vscode/blob/fcd9a8996aca402aa186ad18e5d356ddf0c53e2f/src/vs/base/browser/ui/list/listView.ts.

So, they care about 3d transform to not break subpixel-antialiasing.

I'm not sure about overall Mattermost architecture, but I saw some 3d transform in Platform. Again, I'm not really sure still.

To continue need to force subpixel-antialiasing in the simplest Electron demo, and need to be sure there is no side-effects from the code.

fadeevab commented 7 years ago

(I sometimes press this ridiculous "Close and comment" button)

yuya-oc commented 7 years ago

@fadeevab Thanks for more detailed feedback! It looks the problem is grayscale anti-aliasing described in electron/electron#6344. When I set backgroundColor for Electron's BrowserWindow, I got good anti-aliasing as well as Chrome.

Actually our app uses <webview> to show mattermost/platform, so we need to set backgroundColor as well. But it's not exposed in Electron. So I added a issue. electron/electron#10025

fadeevab commented 7 years ago

Great! I saw this point too, but I set just basic background in CSS and it didn't help. Great you managed to deal with it.

fadeevab commented 7 years ago

Will you add workaround to mattermost or wait for a fix in electron?

yuya-oc commented 7 years ago

Unfortunately there is no workaround we can do in our code. Only Electron's native part can fix this.

fadeevab commented 7 years ago

I got it. Let's wait than, thank you :)

RoyYangS commented 7 years ago

It caused by setting the transparent:true !!!! I have a greate font appr now

fadeevab commented 7 years ago

Great!

yuya-oc commented 7 years ago

@YangShengHaHa I think transparent:true is not used in the app though, how did you apply your fix? I'm wondering if you can make a PR.

6a7070 commented 7 years ago

I'm also interested in a fix for this as well. For me, I avoid using the Windows client because of the lack of focus when I view the text. Is there a solution for this issue? If not, do you know if Electron is actively working to fix it? Thank you!

yuya-oc commented 7 years ago

Hi @6a7070, thanks for your interest. As I wrote at electron/electron#10025, we can know why bad anti-aliasing happens. To solve this, we need to improve Electron source code. But I haven't ever heard Electron team is working for this.

jasonblais commented 7 years ago

On a side note, @YangShengHaHa we'd be curious to hear how you resolved (where did you set transparent:true)? That might benefit others in the community as well if you were able to find a fix for the problem.

RoyYangS commented 7 years ago

@jasonblais @yuya-oc when you create your BrowserWindow , set transparent:false in then option of then BrowserWindow constructor.

yuya-oc commented 7 years ago

Hi @YangShengHaHa, unfortunately I could not find difference even when using transparent: false at the current master branch. In the first place, the default value of transparent is false, though.

If transparent: false truly solves the problem, I think we need to set transparent: false to <webview> not only for the main BrowserWindow because their webContents are independent.

I'm wondering if you could show difference as screenshots. And we welcome when you have a pull request.

fadeevab commented 7 years ago

Hey guys, AFAIK, to activate subpixel (good) anti-aliasing need to set backgroundColor to white in some native code, not transparent: "BrowserWindow shows different rendering when using backgroundColor: '#FFF' in the constructor."

RoyYangS commented 7 years ago

@yuya-oc I can not take picture for our project code because of the security policy, you can write a simple demo and you will get the answer. I consider wherever this is one the cause of font anti... We have the best practice WeLink for electron from Huawei inc. Setting the BrowserWindow transparent to false in main.js resolves our font problem in WeLink ^_^

6a7070 commented 6 years ago

Thanks for all the feedback. I'm still unclear how this might be fixed. Is this something I would need to configure in my own compiled version of the desktop client? I find that I don't use the desktop client because of this issue. Is there anything I can do to help test?

yuya-oc commented 6 years ago

In normal Electron apps, @YangShengHaHa's approach would be good to fix the problem. It's to create non-transparent BrowserWindow in main.js. However it's not enough for us. To show Mattermost webapp, we are using <webview> tag. It has its own renderer process (including background color) apart from the main window. So we need to set non-transparent background for each <webview>, but there are no such APIs in <webview> or its webContents object. So in order to solve the problem, we need to make a improvement for Electron APIs.

amyblais commented 5 years ago

Ticket for tracking: https://mattermost.atlassian.net/browse/MM-14133

devinbinnie commented 2 years ago

Closing old issue, looks like this is working fine. Happy to re-open if we think this is still a problem.