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

fix bbox as per spec #2177

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

omeid
Copy link

@omeid omeid commented Nov 24, 2023

Summary

As per the spec, bbox should not include any transformations, including scaling. It should also not include any control points. This fixes getBBox to give correct results matching Google Chrome, Firefox as per the spec.

  • What issues does the pull request solve? Please tag them so that they will get automatically closed once the PR is merged

  • What is the feature? (if applicable)
    Bug fix.

  • How did you implement the solution?
    Like this, see.

  • What areas of the library does it impact?
    getBBox API.

Test Plan

I build some stuff that works with SVG, the results on web were inconsistent, and digging down I realise it was a react-native-svg.

Some fun fact, there was a surprisingly similar bug related to very confusing named (CGPathGetBoundingBox vs CGPathGetPathBoundingBox) APIs as well.
https://bugzilla.mozilla.org/show_bug.cgi?id=1369904

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS
Android
Web 🔘 *
  • Depends on the host platform. But works fine in major browsers.

Checklist

  • I have tested this on a device and a simulator
  • I added documentation in README.md
  • I updated the typed files (typescript)
  • I added a test for the API in the __tests__ folder

@omeid
Copy link
Author

omeid commented Dec 5, 2023

cc @msand

Since you have laid out the initial work for this.

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

Successfully merging this pull request may close these issues.

None yet

1 participant