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

When we use getBoundingClientRect(), we should account for borders. #11

Open
diarmidmackenzie opened this issue Mar 2, 2024 · 1 comment

Comments

@diarmidmackenzie
Copy link
Owner

diarmidmackenzie commented Mar 2, 2024

When a screen element being rendered to has a border, these aren't allowed for in the render viewport calculations.

This means that rendering might be slightly off-center vs. what's expected, especially in cases where the element being rendered to has wide or asymmetric borders.

I have a fix for this, but I am concerned about the possible performance impact.

@diarmidmackenzie
Copy link
Owner Author

Branch fix-border-issue contains a fix for this that is functionally correct.

My concern is whether there are performance issues extracting the border information using getComputedStyles every frame. Also, though I hadn't noticed this before, in both old & new code, we create a new DomRect object every render, which also isn't aligned with performance best practice due to resulting garbage collection issues.

This might all be a non-issue, assuming:

  • a relatively small number of secondary cameras...
  • only applies when rendering to screen, which means we aren't in VR, and so very high FPS (72 or 90) is likely not required...

In any case, I want to merge #12 before creating a PR from this branch...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant