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

Shape.bounds returns incorrect rectangle #623

Open
mhdhejazi opened this issue Sep 25, 2019 · 4 comments
Open

Shape.bounds returns incorrect rectangle #623

mhdhejazi opened this issue Sep 25, 2019 · 4 comments
Assignees
Labels
Milestone

Comments

@mhdhejazi
Copy link

Consider the following simple shape:
<path d="M160 80.3848C160 187.051 0 187.051 0 80.3848C0 -26.2819 160 -26.2819 160 80.3848Z" fill="#C4C4C4"/>
image

As you can see the size is 160x160, but when I call Shape.bounds it returns:
x: 0.000000, y: -26.281900, w: 160.000000, h: 213.332900

If we check the curves of this path closely we can understand what's happening. The returned rectangle is bigger because it also includes the control points:
image

I checked how the bounds are calculated and found out that the native method CGContext. boundingBoxOfPath is the culprit since it calculates the bounding box for all the points including the control points:

CGContext.boundingBoxOfPath

The bounding box is the smallest rectangle completely enclosing all points in a path, including control points for Bézier cubic and quadratic curves.

While CGPath.boundingBoxOfPath, despite having the exact same name, doesn't include the control points in the calculation:

CGPath.boundingBoxOfPath

The path bounding box is the smallest rectangle completely enclosing all points in the path but not including control points for Bézier and quadratic curves.

I tried using ctx.path?.boundingBoxOfPath instead of ctx.boundingBoxOfPath here:

var rect = ctx.boundingBoxOfPath

and it returned the correct result:
x: 0.000000, y: 0.384775, w: 160.000000, h: 159.999675

Searching for boundingBoxOfPath in this repo I found out that Path.bounds() actually uses the correct method:

override open func bounds() -> Rect {
return toCGPath().boundingBoxOfPath.toMacaw()
}

So I think this method can be used as a workaround until this is fixed (instead of Shape.bounds, one can use Shape.form.bounds()).

@ystrot ystrot self-assigned this Nov 29, 2019
@ystrot ystrot added the bug label Nov 29, 2019
@ystrot ystrot added this to the 0.9.6 milestone Nov 29, 2019
@ystrot ystrot closed this as completed in 1e36894 Nov 29, 2019
@ystrot
Copy link
Member

ystrot commented Nov 29, 2019

Hi @mhdhejazi,

Thanks for detailed description! I've pushed a fix.

@ystrot
Copy link
Member

ystrot commented Nov 29, 2019

Actually it's not that easy. We have simple sample in tests:

<path d="M10,10C40,10,65,10,95,80S150,150,180,80"/>

And on this sample both CGPath.boundingBoxOfPath and CGContext. boundingBoxOfPath works incorrectly (blue is path bounds, red is context bounds):

Screenshot 2019-11-29 at 17 58 56

In this case we need to use context, because at least it returns a bounding box. It's not minimal, but path bounding box completely wrong. So I'll revert this change and we need to wait Apple will fix it.

@ystrot ystrot reopened this Nov 29, 2019
@ystrot ystrot closed this as completed in a649773 Nov 29, 2019
@ystrot ystrot reopened this Nov 29, 2019
@mhdhejazi
Copy link
Author

I see.
But it's confusing that Shape.bounds doesn't return the same value as Path.bounds(). The former returns the bigger rectangle in your example, while the later returns the smaller one.

@ystrot
Copy link
Member

ystrot commented Nov 29, 2019

Agree, but it's the best we can do. In Path.bounds we don't have context, so we need to use path method.

explaineverything pushed a commit to explaineverything/Macaw that referenced this issue Mar 3, 2020
* master: (79 commits)
  Update .gitignore
  Add Package.swift
  Revert "Fix exyte#623: Shape.bounds returns incorrect rectangle"
  Fix exyte#623: Shape.bounds returns incorrect rectangle
  exyte#640 Remove force unwrap in `SVGParser.doubleFromString` method
  Fix crash in parsing pattern
  Formatting issues fix
  Fix gradient stops parsing
  Lint formatting issues fixed
  Formatting fixes
  fix source files of projects, add to macos target
  Enable Catalyst support (fixes exyte#610)
  Fix links in readme
  Update README.md
  Added supported platform - macOS
  Restore method
  Replace test references
  Revert "Add print to tests"
  Trigger build
  Add print to tests
  ...

# Conflicts:
#	Source/render/GroupRenderer.swift
#	Source/svg/SVGParser.swift
#	Source/views/MacawView.swift
@ystrot ystrot modified the milestones: 0.9.6, 0.9.7 Apr 10, 2020
@ystrot ystrot modified the milestones: 0.9.7, 0.9.x Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants