Skip to content

perf: Multiple getBoundingClientRect calls in isFocusable/isVisible #9148

@PascalThuet

Description

@PascalThuet

Description

The isVisible() helper function inside isFocusable() in component.js calls getBoundingClientRect() 4 times on the same element within a single invocation, causing unnecessary layout thrashing.

Current Code (component.js:1916-1927)

function isVisible(element) {
  if ((element.offsetWidth + element.offsetHeight + 
       element.getBoundingClientRect().height +   // 1st call
       element.getBoundingClientRect().width) === 0) {  // 2nd call
    return false;
  }
  const elementCenter = {
    x: element.getBoundingClientRect().left + element.offsetWidth / 2,  // 3rd call
    y: element.getBoundingClientRect().top + element.offsetHeight / 2   // 4th call
  };
  // ...
}

Proposed Fix

Cache the getBoundingClientRect() result once:

function isVisible(element) {
  const rect = element.getBoundingClientRect();
  const width = element.offsetWidth;
  const height = element.offsetHeight;

  if ((width + height + rect.height + rect.width) === 0) {
    return false;
  }
  const elementCenter = {
    x: rect.left + width / 2,
    y: rect.top + height / 2
  };
  // ...
}

Benchmark Results

Metric Current Optimized Improvement
10,000 calls 6.80 ms 4.90 ms 28% faster
Per call 0.68 µs 0.49 µs -0.19 µs

Impact

  • Keyboard navigation: Every Tab/arrow key triggers visibility checks
  • Mobile devices: Layout calculations are 4-10x more expensive
  • Spatial navigation feature: Heavily relies on this function

Environment

  • video.js version: 8.23.6
  • Tested on: Chrome 144, macOS

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions