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

Lists in views are no longer on a separate GPU layer #97498

Closed
knopp opened this issue May 11, 2020 · 9 comments
Closed

Lists in views are no longer on a separate GPU layer #97498

knopp opened this issue May 11, 2020 · 9 comments
Assignees
Labels
candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority list-widget List widget issues regression Something that used to work is now broken verified Verification succeeded

Comments

@knopp
Copy link

knopp commented May 11, 2020

Right now when scrolling project explorer vscode redraws entire window (as can be seen on mac by enabling "Flash screen updates" in Quarz Explorer. This can be rather sluggish on HiDPI screens.

This does not happen when scrolling code editors, as they are using separate layers (triggered using translate:transform3d(0,0,0)) i believe.

Same should be done for panes. The scrolling performance is much better when adding something like "div.pane { translate:transform3d(0,0,0); } or contain:strict.

The only problem I encounter is that newly scrolled items are painted without subpixel antialiasing. I don't know the reason why this happens, but I assume it should be fixable since subpixel aa is working for the main editor.

@sbatten
Copy link
Member

sbatten commented May 11, 2020

/cc @bpasero @deepak1556

@bpasero
Copy link
Member

bpasero commented May 11, 2020

The only problem I encounter is that newly scrolled items are painted without subpixel antialiasing.

Well, that would be a blocker because we heavily invested in supporting subpixel-AA everywhere.

Though I thought the tree/list was different, but I forgot, maybe we removed this. @joaomoreno what is the latest of the list widget using the scroll trick from the editor?

@knopp
Copy link
Author

knopp commented May 11, 2020

Well, that would be a blocker because we heavily invested in supporting subpixel-AA everywhere.

I agree. However given that it works in main editor, perhaps there is a way to determine what exactly is causing chromium to drop subpixel aa for new items and work around it.

The speed improvements when scrolling through a large project on 4K HiDPI screen are pretty significant.

@joaomoreno
Copy link
Member

joaomoreno commented May 12, 2020

I don't think the initial premise true:

recording (4)

Every list, tree and editor is already in its own layer. The lists and trees are currently doing the same as the editors.

Can you use the Paint Flashing tool from the built-in DevTools?

@joaomoreno joaomoreno added the info-needed Issue requires more information from poster label May 12, 2020
@knopp
Copy link
Author

knopp commented May 12, 2020

@joaomoreno, this may be actually Mac specific. It is about how Chromium split the scene in CALayers and which CALayers get updated. There is no easy way on Mac to do partial damage update, so instead Chromium splits the scene in multiple CALayers that can be individually updated.

You can then use Quartz Debug (from additional Xcode tools) and enable "Flash screen updates" to see which part of windows are updated. You can also run vscode with "--show-mac-overlay-borders" to see how the scene is split into multiple CALayers.

And here is where it gets interesting. I actually found just now that this is a regression between 1.45 and 1.44.2. I'm attaching screenshots with -show-mac-overlay-borders. You can see that in 1.44.2 there is a layer border around the explorer, which is not present in 1.45. Thus scrolling explorer refreshes the entire window. I replaced electron from 1.45 inside 1.44.2 and it still works properly, so it looks like vscode regression instead of electron one.

borders - 1 44 2borders - 1 45

@knopp
Copy link
Author

knopp commented May 12, 2020

Okay, so the regression is that in 1.45, div.monaco-list-rows no longer has transform:translate3d(0px, 0px, 0px)

@bpasero bpasero added the important Issue identified as high-priority label May 12, 2020
@bpasero
Copy link
Member

bpasero commented May 12, 2020

#94195 ?!

@bpasero
Copy link
Member

bpasero commented May 12, 2020

@rebornix @sbatten this probably qualifies as candidate. it looks like options.transformOptimization was added which maybe you forgot to pass in for lists?

@bpasero bpasero changed the title Use separate layer for workspace explorer and other panes Lists in views are no longer on a separate GPU layer May 12, 2020
@bpasero
Copy link
Member

bpasero commented May 12, 2020

@knopp excellent analysis btw, kudos. Great catch 👍

@bpasero bpasero added candidate Issue identified as probable candidate for fixing in the next release regression Something that used to work is now broken and removed info-needed Issue requires more information from poster labels May 12, 2020
@bpasero bpasero added this to the April 2020 Recovery milestone May 12, 2020
@bpasero bpasero added the list-widget List widget issues label May 12, 2020
rebornix added a commit that referenced this issue May 13, 2020
…lt-translate3d

Fix #97498. Fallback to default value for translate3d
@mjbvz mjbvz added verified Verification succeeded and removed verified Verification succeeded labels May 13, 2020
@sbatten sbatten added the verified Verification succeeded label May 14, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
candidate Issue identified as probable candidate for fixing in the next release important Issue identified as high-priority list-widget List widget issues regression Something that used to work is now broken verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants