Make isVisible 10% faster by avoiding unneeded getComputedStyle() calls

[erikrose]

Emilio had a great idea. See this mail thread.

--

Err, yeah, I of course wanted to say "not visible".

If the node is zero size but has overflow, and you care about that, then you can check !element.getClientRects().length as the check instead.

-- Emilio

On 2/2/21 22:44, Erik Rose wrote:

Although, now that I think about it, what if a node's style.overflow is "visible"? Won't your suggested code call a node invisible even though its content can be seen? If so, I'm afraid the shortcut doesn't avail, because we have to go get the computed style to tell what the overflow property is, as we do around https://github.com/mozilla/fathom/blob/49d678dc2a44f35e5ad8233059ebbbe8feab3be6/fathom/utilsForFrontend.mjs#L478 https://github.com/mozilla/fathom/blob/49d678dc2a44f35e5ad8233059ebbbe8feab3be6/fathom/utilsForFrontend.mjs#L478. Hoping I'm wrong. :-) Erik Big help, potentially! I'll ticket it to try and benchmark. Did you really mean "return true", or did you mean "false" (meaning "not visible")?

Thanks! Erik

On Jan 19, 2021, at 14:28 , Emilio Cobos Álvarez <emilio@mozilla.com mailto:emilio@mozilla.com> wrote:

~10% of the profile is under ResolveStyleLazily. That means that you're calling getComputedStyle in a display: none subtree.

Perhaps a bit counter-intuitively, I think you can basically eliminate that by checking something like:

let rect = ancestor.getBoundingClientRect(); if (rect.width == 0 || rect.height == 0) return true;

That will also allow you to fix the style.{width,height} == '0' checks, which are bogus (it should use '0px'), and which can be removed with the snippet I posted above.

Hope it helps.

[RhnSharma]

I would like to take this one.

[afif1400]

I would like to work on this.