ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.1k stars 13.51k forks source link

bug: webkit, style recalculations on hover with fixed slot #24386

Open tlaverdure opened 2 years ago

tlaverdure commented 2 years ago

Prerequisites

Ionic Framework Version

Current Behavior

This issue attempts to address performance issues with ion-content, a continuation of #24359.

There are some performance issues with ion-content as more child DOM nodes are added. Any interaction with the pointer causes Styles Invalidated and Styles Recalculated events to occur that are very slow. I'm not sure if this is an issue with:

Expected Behavior

The styles required for ion-content should not introduce performance issues as the number of DOM nodes increases within the component's slot.

Steps to Reproduce

In Safari, using developer tools you should be able to use the "Timeline" to inspect the CPU usage of this page after it has loaded:

https://angular-ivy-7he8g8.stackblitz.io

By just moving my mouse and clicking around CPU is maxed out. All related to styles and painting on the main thread.

https://user-images.githubusercontent.com/1731025/145899511-181cb2b4-89bd-45da-aaca-19e98bd2e9d5.mov

Now if I delete the <style> tag within the <ion-content> shadow DOM, the overall CPU usage is much less.

https://user-images.githubusercontent.com/1731025/145899583-013d680a-0ae5-48db-84aa-84081b103b8c.mov

Code Reproduction URL

https://stackblitz.com/edit/angular-ivy-7he8g8

Ionic Info

latest

Additional Information

No response

liamdebeasi commented 2 years ago

Thanks for following up on this. At first glance, this looks related to https://github.com/ionic-team/ionic-framework/issues/22951 (though not necessarily related to CSS variables). I'll take a closer look and follow up here.

liamdebeasi commented 2 years ago

Took a closer the look. The issue appears to be due to the fixed slot. Adding a position: absolute inside of the Shadow DOM causes WebKit to recalculate the styles whenever you mouse over an interactive element such as an anchor element.

tlaverdure commented 2 years ago

Thanks! After testing this further I can see performance degrading with the inclusion of css variables. In addition, performance is exacerbated by shadow DOM pseudo selectors. When I comment out all the :host() and ::slotted() selectors in the stylesheet CPU performance rests at <= 20%.

/* :host {
    --background:var(--ion-background-color, #fff);
    --color:var(--ion-text-color, #000);
    --padding-top:0px;
    --padding-bottom:0px;
    --padding-start:0px;
    --padding-end:0px;
    --keyboard-offset:0px;
    --offset-top:0px;
    --offset-bottom:0px;
    --overflow:auto;
    display: block;
    position: relative;
    -ms-flex: 1;
    flex: 1;
    width: 100%;
    height: 100%;
    margin: 0 !important;
    padding: 0 !important;
    font-family: var(--ion-font-family, inherit);
    contain:size style
}

:host(.ion-color) .inner-scroll {
    background: var(--ion-color-base);
    color:var(--ion-color-contrast)
}

:host(.outer-content) {
    --background:var(--ion-color-step-50, #f2f2f2)
} */

#background-content {
    left:0px;
    right:0px;
    top:calc(var(--offset-top) * -1);
    bottom:calc(var(--offset-bottom) * -1);
    position:absolute;
    background:var(--background)
}

.inner-scroll {
    left: 0px;
    right: 0px;
    top:calc(var(--offset-top) * -1);
    bottom:calc(var(--offset-bottom) * -1);
    padding-left:var(--padding-start);
    padding-right:var(--padding-end);
    padding-top:calc(var(--padding-top) + var(--offset-top));
    padding-bottom:calc(var(--padding-bottom) + var(--keyboard-offset) + var(--offset-bottom));
    position: absolute;
    color:var(--color);
    -webkit-box-sizing: border-box;
    box-sizing: border-box;
    overflow: hidden;
    -ms-touch-action: manipulation;
    touch-action:manipulation
}

@supports ((-webkit-margin-start: 0) or(margin-inline-start: 0)) or(-webkit-margin-start: 0) {
    .inner-scroll {
        padding-left:unset;
        padding-right:unset;
        -webkit-padding-start:var(--padding-start);
        padding-inline-start:var(--padding-start);
        -webkit-padding-end:var(--padding-end);
        padding-inline-end:var(--padding-end)
    }
}

.scroll-y, .scroll-x {
    -webkit-overflow-scrolling: touch;
    z-index:0;
    will-change:scroll-position
}

.scroll-y {
    overflow-y: var(--overflow);
    overscroll-behavior-y:contain
}

.scroll-x {
    overflow-x: var(--overflow);
    overscroll-behavior-x:contain
}

.overscroll::before, .overscroll::after {
    position: absolute;
    width: 1px;
    height: 1px;
    content: ""
}

.overscroll::before {
    bottom:-1px
}

.overscroll::after {
    top:-1px
}

/* :host(.content-sizing) {
    display: -ms-flexbox;
    display: flex;
    -ms-flex-direction: column;
    flex-direction: column;
    min-height: 0;
    contain:none
}

:host(.content-sizing) .inner-scroll {
    position: relative;
    top: 0;
    bottom: 0;
    margin-top: calc(var(--offset-top) * -1);
    margin-bottom:calc(var(--offset-bottom) * -1)
} */

.transition-effect {
    display: none;
    position: absolute;
    left: -100%;
    width: 100%;
    height: 100vh;
    opacity: 0;
    pointer-events:none
}

.transition-cover {
    position: absolute;
    right: 0;
    width: 100%;
    height: 100%;
    background: black;
    opacity:0.1
}

.transition-shadow {
    display: block;
    position: absolute;
    right: 0;
    width: 10px;
    height: 100%;
    background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAgCAYAAAAIXrg4AAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyhpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuNi1jMTQ1IDc5LjE2MzQ5OSwgMjAxOC8wOC8xMy0xNjo0MDoyMiAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENDIDIwMTkgKE1hY2ludG9zaCkiIHhtcE1NOkluc3RhbmNlSUQ9InhtcC5paWQ6MTE3MDgzRkQ5QTkyMTFFOUEwNzQ5MkJFREE1NUY2MjQiIHhtcE1NOkRvY3VtZW50SUQ9InhtcC5kaWQ6MTE3MDgzRkU5QTkyMTFFOUEwNzQ5MkJFREE1NUY2MjQiPiA8eG1wTU06RGVyaXZlZEZyb20gc3RSZWY6aW5zdGFuY2VJRD0ieG1wLmlpZDoxMTcwODNGQjlBOTIxMUU5QTA3NDkyQkVEQTU1RjYyNCIgc3RSZWY6ZG9jdW1lbnRJRD0ieG1wLmRpZDoxMTcwODNGQzlBOTIxMUU5QTA3NDkyQkVEQTU1RjYyNCIvPiA8L3JkZjpEZXNjcmlwdGlvbj4gPC9yZGY6UkRGPiA8L3g6eG1wbWV0YT4gPD94cGFja2V0IGVuZD0iciI/PmePEuQAAABNSURBVHjaYvz//z8DIxAwMDAwATGMhmFmPDQuOSZks0AMmoJBaQHjkPfB0Lfg/2gQjVow+HPy/yHvg9GiYjQfjMbBqAWjFgy/4hogwADYqwdzxy5BuwAAAABJRU5ErkJggg==);
    background-repeat: repeat-y;
    background-size:10px 16px
}

/* ::slotted([slot=fixed]) {
    position: absolute;
    -webkit-transform: translateZ(0);
    transform: translateZ(0)
} */

I've also tested commenting out just the properties within these rules but CPU usage still increases.

:host {
    /*--background:var(--ion-background-color, #fff);
     --color:var(--ion-text-color, #000);
    --padding-top:0px;
    --padding-bottom:0px;
    --padding-start:0px;
    --padding-end:0px;
    --keyboard-offset:0px;
    --offset-top:0px;
    --offset-bottom:0px;
    --overflow:auto;
    display: block;
    position: relative;
    -ms-flex: 1;
    flex: 1;
    width: 100%;
    height: 100%;
    margin: 0 !important;
    padding: 0 !important;
    font-family: var(--ion-font-family, inherit);
    contain:size style */
} 

I also tried commenting out just the :slotted rule with the position: absolute property. This seems to help some with mouse movement, but once you start clicking CPU increases to > 30% and interactions seem sluggish.

liamdebeasi commented 2 years ago

Thanks for the added information. The clicking issue when CSS Variables are enabled is probably https://github.com/ionic-team/ionic-framework/issues/22951.

Is this issue impacting an production applications/websites for you?

tlaverdure commented 2 years ago

Not in production yet but I've been blocked for a few of weeks trying to figure out this issue. My app is snappy while testing in Chrome but pretty much unusable on iOS devices 😅

Just to reiterate what I'm seeing with the selectors check out this video:

https://user-images.githubusercontent.com/1731025/146028426-fc7569b2-5772-4003-b1da-01beb463339d.mov

liamdebeasi commented 2 years ago

Thanks! I followed up with the WebKit team: https://bugs.webkit.org/show_bug.cgi?id=234297

I will reply here when I have more to share on this.

tlaverdure commented 2 years ago

Would converting ion-content to use scoped: true instead of shadow: true make any difference in performance with CSS variables?

liamdebeasi commented 2 years ago

I do not think that would make any difference with CSS Variables. I think it would get rid of the slot issue, but that is mainly because slots as a feature do not exist outside of the Shadow DOM at the moment.

Regardless, this is a breaking change and we can only do those during major releases.

tlaverdure commented 2 years ago

Ok. The only idea I have left is to create my own content component. Do you think it would be acceptable to PR a change to the usage of el.closest('ion-content') to el.closest('ion-content,[ion-content]'):

https://github.com/ionic-team/ionic-framework/search?q=el.closest%28%27ion-content%27%29

This interface should get me going, I think 😄 https://github.com/ionic-team/ionic-framework/blob/412e5f168e971994c9eec8152b72821ba32bfc2d/core/src/components.d.ts#L660-L713

If you have any other better ideas let me know!