[CLOSED] [MAC] Enabling back sub pixel antialiasing for code view

[core-ai-bot]

Issue by nethip
Tuesday Jun 09, 2015 at 10:26 GMT
Originally opened as adobe/brackets#11235

---

Defaulting the code view antialiasing to sub-pixel which can be overridden using a new preference fontSmoothing. This preference takes subpixel-antialiased or antialiased. With these changes, the UI would have gray scale anti-aliasing but the code view container editor-holder, would have subpixel-antialiasing on.

The reason to disable subpixel antialiasing for UI is that, with subpixel AA on, SourceSansPro is not rendering fine on low res monitors in Brackets UI.

Historically sub pixel AA was turned off to circumvent the flickering bug. Looks like this is being fixed in CEF2171 as I am unable to repro this bug anymore.

I tested this PR on variety of hardware (retina, non-retina, external monitor) and the code rendering is a little crisper now on MAC.

---

nethip included the following code: https://github.com/adobe/brackets/pull/11235/commits

[core-ai-bot]

Comment by nethip
Tuesday Jun 09, 2015 at 10:31 GMT

---

Fixes adobe/brackets#11013, adobe/brackets#4640 and adobe/brackets#10169

[core-ai-bot]

Comment by nethip
Tuesday Jun 23, 2015 at 04:07 GMT

---

@busykai@MarcelGerber Would you mind reviewing this PR? This is targeted for 1.4.

[core-ai-bot]

Comment by MarcelGerber
Tuesday Jun 23, 2015 at 16:27 GMT

---

Looks good to me otherwise.

[core-ai-bot]

Comment by nethip
Tuesday Jun 23, 2015 at 17:23 GMT

---

@MarcelGerber Thanks for taking a look at this PR! I will incorporate the changes you have suggested and update this PR.

[core-ai-bot]

Comment by nethip
Thursday Jun 25, 2015 at 10:27 GMT

---

After analyzing how fonts render on both retina and non-retina and also comparing the rendering with other apps I have come to the following conclusion.

1.  Editors like Sublime Atom and Visual Studio code enable sub pixel rendering by default, irrespective of whether the display is retina or non-retina.
   

2.  Some editors, like Sublime, allow users to override this default subpixel rendering. In sublime, “font_options”preference can be used to control the code rendering to use sub pixel AA or gray scale AA. This override is limited to just the code area alone. The rest of the UI is still using sub-pixel rendering.
   

So the logical conclusion I have arrived at was that, we should be enabling sub-pixel rendering by default and give an option to user to override this behavior. So I went ahead and added a new preference “fontSmoothing”, which would take “subpixel-antialiased” or “antialiased” as the values for this preference.

Now with this in mind I went ahead and enabled subpixel-AA for the entire app(apply –webkit-font-smoothing:”subpixel-antialised” to the body tag). After that I looked at how the overall rendering looked like. The code area was looking a bit more crisp.

But the problem was with other parts of the UI like project tree, find in files pane, JS linting pane e.t.c. I could immediately see extra artifacts being rendered. The following screenshot was taken on an external monitor with a resolution of 1680 x 1050, connected to Mac Pro, with subpixel-AA on.

This is the same rendered with gray pixel on AA, which is the current scheme on master.

I am putting the screenshots of Brackets with subpixel-AA on and off. The first one is with subpixel-AA on. Just by glancing we can see that the first one(subpixel-AA on) has some extra pixels which is making it look a little out of place.

I tried substituting the UI font SourceSansPro with system fonts like Lucida Grande and Helvetica Neue and I saw that using system fonts, the fonts render a little better. Looks like SourceSansPro is not rendering fine with text color #ffffff on dark backgrounds after turning on subpixel-AA. I checked on how this is with other apps and found out that Sublime, Atom and Visual Studio Code use system default fonts, which render relatively well for subpixel-AA on cases. Also none of them uses text with color #ffffff and that could be the reason why they text is rendering fine in other apps as white text on black background is producing artifacts even with other apps.

So with the above test results, I went ahead and added subpixel-AA only to the code area. (to #editor.holder alone). Now the following screenshot is the one where subpixel-AA is enabled only to the code area.

[core-ai-bot]

Comment by nethip
Thursday Jun 25, 2015 at 10:51 GMT

---

Tagging@larz0 for his comments on enabling sub pixel AA only for the code area.

[core-ai-bot]

Comment by busykai
Thursday Jun 25, 2015 at 15:53 GMT

---

@nethip, I'm wondering if brackets.js is the right place to do this. Perhaps either view/MainViewManager.js or view/WorkspaceManager.js are more appropriate (MainViewManager.js more so, WorkspaceManager.js manages resizing, really). Both react to htmlReady and manipulate #editor-holder.

Also, if moved to another module, I'd reduce the use of string literals and define modules vars for values and names.

[core-ai-bot]

Comment by MarcelGerber
Friday Jun 26, 2015 at 11:38 GMT

---

Maybe the place where other font prefs like font.fontSize are defined is best.
Can't look it up right now.

[core-ai-bot]

Comment by nethip
Saturday Jun 27, 2015 at 08:15 GMT

---

@MarcelGerber I could move the preference to some place where font prefs are getting initialised.

[core-ai-bot]

Comment by nethip
Saturday Jun 27, 2015 at 08:16 GMT

---

@busykai I think you are right about the code to be present in MainViewManager.js. I will move it to that place.

[core-ai-bot]

Comment by nethip
Saturday Jun 27, 2015 at 08:21 GMT

---

@busykai@MarcelGerber While I was unit testing with this change on MAC, the text now looks crisper with white text on black background. However it looks a little fuzzy on black text on white background. After doing some thinking I came up with this idea of why not add a new entry in the metadata section of themes. That way theme authors can decide on whether they would want the AA to be sub pixel or grayscale. That way we could then enable sub pixel AA only for our default dark theme. What do you guys think about this?

@MiguelCastillo Do you think it is a good idea to add another property to the package.son that instructs brackets to either enable sub pixel AA or grayscale AA?

CC@ryanstewart

[core-ai-bot]

Comment by MiguelCastillo
Monday Jun 29, 2015 at 15:22 GMT

---

@nethipn I had implemented this in a PR a while ago. adobe/brackets#8635... You can checkout the thread. So my thoughts on this now are that we can just create an extension for this instead of putting it in core. What do you think?

[core-ai-bot]

Comment by MiguelCastillo
Monday Jun 29, 2015 at 15:24 GMT

---

I can create the extension and rope you guys in for review if you would like.

[core-ai-bot]

Comment by nethip
Monday Jun 29, 2015 at 18:15 GMT

---

@MiguelCastillo I checked out your PR #8635. So essentially you are doing what I was thinking about in my latest comment!

I am curious to know what you would be packaging as an extension. Even if we decide to go with the code, that you are planning to package as an extension, IMO that should be added to the core. My understanding of an extension is something that the app wills still function as is, even if that extension is disabled/deleted. In this case, the rendering itself is broken and I am thinking it must in the core. But this is just my opinion and if we have a strong reason to ship this as an extension, that is fine too.

[core-ai-bot]

Comment by MarcelGerber
Wednesday Jul 01, 2015 at 15:07 GMT

---

@nethip I don't have a Mac, so I can't repro myself, but is the rendering on dark backgrounds really that much worse than it is on light ones?

The thing is, I can think of scenarios like "The font looks awful with your theme", where you can't really blame the developer because he presumably either didn't know or didn't think about including the antialiasing pref in his theme (maybe just because they don't have a Mac themself).

[core-ai-bot]

Comment by MiguelCastillo
Wednesday Jul 01, 2015 at 16:12 GMT

---

@nethip we can always put things in core... Not a problem. But the reason this was not put into core in the first place is because it's not a setting that applies across platforms. So it didn't seem like a good approach for us to add a setting that only worked on Mac and would potentially cause confusion when users in different platforms wouldn't see a difference when switching the setting. This whole trouble was not worth our effort of including it in core to solve a relatively niche problem.

I am not sure what you mean by rendering is broken... If anti-aliasing is breaking rendering, then we fix it and don't make it an option to break it again.

The extension would possibly be JS that exposes a way to switch the anti-aliasing setting on/off via preferences, a small UI, or even a menu setting similar to settings like Word Wrapping. The extension would have in the repo a large note stating that it is ONLY for Mac.

• I myself run a tiny css that disables font smoothing, which is what I expect the extension to do.

html, body {
   -webkit-font-smoothing: subpixel-antialiased !important;
}

[core-ai-bot]

Comment by nethip
Wednesday Jul 01, 2015 at 17:44 GMT

---

@MiguelCastillo

we can always put things in core... Not a problem. But the reason this was not put into core in the first place is because it's not a setting that applies across platforms. So it didn't seem like a good approach for us to add a setting that only worked on Mac and would potentially cause confusion when users in different platforms wouldn't see a difference when switching the setting. This whole trouble was not worth our effort of including it in core to solve a relatively niche problem.

We could make the preference name mac specific, something like font.fontSmoothingMac, making it easier for the user to figure out that this setting is mac specific. Also now that we have code hints for preferences, we could add Mac only in the description.

I am not sure what you mean by rendering is broken... If anti-aliasing is breaking rendering, then we fix it and don't make it an option to break it again.

Looks like people who are on Mac are having a real bad experience with gray scale AA, especially with white text on black background inside Brackets. This is even becoming a deciding factor for users to either switch with Brackets or migrate to other editors. So this is a very serious problem and I am afraid there is no generic solution to this. I have seen that with sub pixel AA enabled on black text on white background, the fonts are looking a little washed out. And that is where I was thinking of making this specific to a theme. But I just proposed an idea. It could be a bad idea too.

html, body {
   -webkit-font-smoothing: subpixel-antialiased !important;
}

As I mentioned in my analysis above, the problem with the above code snippet is that this enables sub pixel AA for the entire app. But the non-code UI is looking washed out with that. So we might want to enable this only for the code area and that is why I am changing #editor-holder style alone.

[core-ai-bot]

Comment by nethip
Wednesday Jul 01, 2015 at 17:51 GMT

---

@MarcelGerber I understand that the theme developers now have to worry about whether to do this use the new setting, if we decide to add it to the theme. Now I am thinking maybe this is a not a good idea to add it as a theme setting. How about just changing sub pixel AA for dark theme alone? That should fix all the issues that I have put forth above.

[core-ai-bot]

Comment by MiguelCastillo
Wednesday Jul 01, 2015 at 17:57 GMT

---

@nethip So with users on Mac complaining that AA is causing a problem, then I would pick the default that works consistently across all platforms... Meaning, pick a default that looks/behaves the same across windows/linux/mac, which is subpixel. I considered doing subpixel just for dark themes, but I strongly believe that providing users with a consistent experience is more important. I think it would be a bit odd if people saw different font rendering for light themes vs dark themes.

I probably wouldn't provide an option to change it back - but this is less important to argue about so I can go either way.

[core-ai-bot]

Comment by nethip
Wednesday Jul 01, 2015 at 18:26 GMT

---

@MiguelCastillo Even I am thinking why don't we just enable it by default.
Users anyway have a preference, which they could use to go back to grayscale AA.

[core-ai-bot]

Comment by nethip
Thursday Jul 02, 2015 at 11:05 GMT

---

@busykai@MarcelGerber @MiguelCastillo I have incorporated all the code review comments. The PR is up for review now. Please take a look at it when you get a chance. Thanks!

[core-ai-bot]

Comment by nethip
Friday Jul 03, 2015 at 18:12 GMT

---

@MarcelGerber This is up for review again.

[core-ai-bot]

Comment by nethip
Thursday Jul 09, 2015 at 06:29 GMT

---

@busykai@MarcelGerber Did you get a look at the updated code in this PR? As this is targeted for 1.4, we need to send the strings for translation and I would really appreciate if we could merge this into master as early as possible.

@prafulVaishnav.

[core-ai-bot]

Comment by rawat11
Thursday Jul 09, 2015 at 06:55 GMT

---

@nethip I did some testing with new SP AA, The editor part seemed more readable specially in the black background. But some of the UI elements are not looking crisp (as they are in Windows) ( Eg. Lines and Column info shown on the bottom side of editor) Can we do something on that front ?

The text in white background looks totally washed away. I think darken them a bit might make them look better.

[core-ai-bot]

Comment by nethip
Thursday Jul 09, 2015 at 07:19 GMT

---

@rawat11 Thanks for testing it out! As discussed on this thread, we would go with subpixel AA for all the themes. Users now have a preference to turn this off. And also rendering is not in our control to darken the text as it is completely managed by chrome. AA is the only option using which we can control rendering to some extent.

Could you change the font from Source Code Pro to some system font and update us, how it is looking with lighter theme. Also would you mind checking the same in other editors like Sublime Text and Atom (You might want to change the font and the theme in these editors to look like what it is in Brackets so that we have an apples to apples comparison)

Regarding the UI elements: No change has been done there, so the rendering should be as it is on master.

[core-ai-bot]

Comment by rawat11
Thursday Jul 09, 2015 at 09:53 GMT

---

@nethip I tested with some system fonts and it did not look much of the difference on Black background

With lighter theme (AA on)

With Sub pixel AA on

White theme looks better with SP AA

Also, I tried the same on Atom, but i could only change the theme, could not find the option for font

I guess Atom has SP AA on by default

According to me White themes aren't looking that good with many of the system fonts but changing it to SP AA makes it look a bit better but still not unto the par. With Black it looks good and SP is definitely a good option to move to.

For the UI, I think we have maintained Gray AA but is there no option to make some change on this, it would only add to readability ad well as rendering

[core-ai-bot]

Comment by prafulVaishnav
Friday Jul 10, 2015 at 11:30 GMT

---

@nethip I think there are some conflicts.. Can you please resolve it..

[core-ai-bot]

Comment by nethip
Friday Jul 10, 2015 at 17:07 GMT

---

@prafulVaishnav I will resolve the conflict and merge the PR.
I have added this waffle card to track adding unit tests to this PR.

[core-ai-bot]

Comment by nethip
Friday Jul 10, 2015 at 18:36 GMT

---

@prafulVaishnav I have resolved all the issues and this PR is now merged 😄