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

High CPU usage when using variant-form-material in a each statement #1805

Closed
djdembeck opened this issue Jul 31, 2023 · 3 comments · Fixed by #1807
Closed

High CPU usage when using variant-form-material in a each statement #1805

djdembeck opened this issue Jul 31, 2023 · 3 comments · Fixed by #1807
Labels
bug Something isn't working

Comments

@djdembeck
Copy link
Contributor

djdembeck commented Jul 31, 2023

Current Behavior

In a table-based form of repeated rows of inputs, all 3 columns use class="input variant-form-material". When the form is loaded, there is a visual issue on the page, the browser slows down, and my CPU meter goes to 100%. If I remove variant-form-material only, the CPU usage goes back to normal on that page.

Expected Behavior

Using variant-form-material on a repeated input form, should have no affect on performance, compared to standard styling.

Steps To Reproduce

  1. Create a table, with 3 headers. Add 3 inputs accordingly, with
<td class="table-cell-fit">
    <input type="text" class="input variant-form-material text-sm" />
</td>

structure for each td. In my test case, I am using this in a table with 35 items (on an edit form), within a Stepper component.
3. Load the form page, specifically a form that contains data for the 35 td elements.
4. Notice CPU spike.

Link to Reproduction / Stackblitz

No response

More Information

No response

@djdembeck djdembeck added the bug Something isn't working label Jul 31, 2023
@endigo9740
Copy link
Contributor

endigo9740 commented Jul 31, 2023

@djdembeck the Material styled variants are handled with CSS, per:

https://github.com/skeletonlabs/skeleton/blob/dev/packages/skeleton/src/lib/styles/elements/forms.css#L270

Screenshot 2023-07-31 at 11 01 24 AM

That's it, that's all that occurs when you implement those classes - some additional styles are added.

The only style that might have an affect on performance would be backdrop-blur, which provides a blur effect to the background of the input element. We implement this to help visibility on busy backgrounds. When you're talking tables this means you could easily have dozens and dozens of instances of elements using this style.

So I'd recommend the following:

  1. Shorterm - I'd suggest copying these CSS classes locally into your app.postcss, giving them a unique name, and testing if without the backdrop blur is functional. If so we know the culprit.
  2. Longterm - if this is the issue, let us know and we can consider dropping the backdrop blur from this element. I think it was well intended, but likely unnecessary.

@djdembeck
Copy link
Contributor Author

djdembeck commented Jul 31, 2023

Thanks for the quick response!

  1. Shorterm - I'd suggest copying these CSS classes locally into your app.postcss, giving them a unique name, and testing if without the backdrop blur is functional. If so we know the culprit.

You were correct, removing only backdrop-blur resolves the issue.

It may be worth mentioning as well, this seems to be a Blink (Vivaldi/Chrome) engine issue. I tested on Firefox, and with backdrop-blur enabled, there is no slowdown (at least to the magnitude Vivaldi had issue).

@endigo9740
Copy link
Contributor

Thank you for confirming @djdembeck, then the role of this ticket will be to remove that in an upcoming patch.

Yeah browser each have their own ways to go about this. It may be they lean on the CPU more than the GPU for this effect in particular. Hopefully that changes in the future. The latter would likely be more performant when a good GPU is present!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants