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

List, Tree: Make sure sub-pixel antialiasing works across all lists and trees #84715

Closed
27 tasks done
alexdima opened this issue Nov 13, 2019 · 10 comments
Closed
27 tasks done
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug lcd-text-rendering list-widget List widget issues verified Verification succeeded
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Nov 13, 2019

@joaomoreno says:

TL;DR Are you a list/tree user? You need to provide a background color to the list/tree in order to preserve sub-pixel anti-aliasing in Windows and Linux.

Recent findings by @alexandrudima and @bpasero uncovered that sub-pixel anti-aliasing gets disabled when a layer is transparent. The list/tree automatically creates a new rendering layer for scrolling performance. So, in order to preserve sub-pixel AA, the root DOM element of the layer must have a background color. That background color depends on its placement in the workbench.

Example: the Explorer should just forward the sidebar background color:

listBackground: SIDE_BAR_BACKGROUND

Note: Only opaque colors work here, since passing in a transparent color would defeat the whole purpose and would potentially place the same transparent background color twice, making it darker. If a theme or user eventually passes in a transparent color, the list/tree will ignore it and print the following warning to the devtools:

List with id 'list_id_4' was styled with a non-opaque background color. This will break sub-pixel antialiasing.

Note 2: macOS doesn't have sub-pixel AA. This only affects Linux and Windows.


Workbench List/Tree Users

Non-Workbench List/Tree Users

These trees do not benefit from the workbench facilities. They use their own stylers. The listBackground needs to be added to those styles.


Good: Sub-pixel AA

image

Bad: Normal AA

image


Original issue by @alexandrudima : fyi @bpasero
  • take a screenshot on Windows of a tree, like the Explorer

  • zoom a lot

  • observe it uses greyscale text rendering:
    image

  • if I make the following change in listView.ts:
    image

  • then, the text renders with subpixel AA:
    image

TL;DR: From what I could tell, as soon as you have a separate layer, the layer dom node itself must have an opaque background, at all times. If it gets transparent even for a split second, it will permanently render with grayscale text. More of my findings documented here

@joaomoreno
Copy link
Member

This needs to be set as a non-optional tree configuration and needs adoption from every tree user.

@joaomoreno joaomoreno added this to the November 2019 milestone Nov 14, 2019
@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug list-widget List widget issues labels Nov 14, 2019
@joaomoreno joaomoreno changed the title Text inside the tree renders with greyscale AA List, Tree: Make sure sub-pixel antialiasing works across all lists and trees Nov 18, 2019
joaomoreno added a commit that referenced this issue Nov 18, 2019
joaomoreno added a commit that referenced this issue Nov 18, 2019
joaomoreno added a commit that referenced this issue Nov 18, 2019
jrieken added a commit that referenced this issue Nov 18, 2019
@isidorn isidorn removed their assignment Nov 18, 2019
@isidorn
Copy link
Contributor

isidorn commented Nov 18, 2019

Thanks a lot for pushing on this.
I have adopted it across my trees.

@isidorn
Copy link
Contributor

isidorn commented Nov 18, 2019

@joaomoreno These changes actually seem to break the tree selection.
When I run out of source the blue tree selection is no longer rendered.
For example just start vscode and click on the explorer.

@joaomoreno
Copy link
Member

@isidorn Pushed fix for it.

jrieken added a commit that referenced this issue Nov 18, 2019
@jrieken jrieken removed their assignment Nov 18, 2019
chrmarti added a commit that referenced this issue Nov 18, 2019
@chrmarti chrmarti removed their assignment Nov 18, 2019
@bpasero
Copy link
Member

bpasero commented Nov 18, 2019

I adopted for notifications, however requires more work because notifications use drop shadows. I opened #85043 as follow up.

@bpasero bpasero removed their assignment Nov 18, 2019
sandy081 added a commit that referenced this issue Nov 18, 2019
@sandy081
Copy link
Member

@sandy081 sandy081 removed their assignment Nov 18, 2019
@rebornix
Copy link
Member

@rebornix rebornix removed their assignment Nov 18, 2019
joaomoreno added a commit that referenced this issue Nov 19, 2019
@joaomoreno
Copy link
Member

@roblourens I've fixed search and settings. Create this additional one for settings: #85106

@joaomoreno
Copy link
Member

joaomoreno commented Nov 19, 2019

@alexr00 I've fixed custom views: it was pretty straightforward, just copied the other viewlet views.

joaomoreno added a commit that referenced this issue Nov 19, 2019
@bpasero bpasero mentioned this issue Nov 19, 2019
2 tasks
@Tyriar
Copy link
Member

Tyriar commented Dec 4, 2019

I'm verifying now, I'll create new more targeted issues for any parts that aren't rendering correctly since this issue contains so much

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug lcd-text-rendering list-widget List widget issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests