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 iPhone X frame #15

Merged
merged 1 commit into from
Jun 13, 2018
Merged

Fix iPhone X frame #15

merged 1 commit into from
Jun 13, 2018

Conversation

jacobp100
Copy link
Contributor

Rather than drawing orthogonal lines from the centre to measure the screen size, this uses a flood fill algorithm, and measures the size of the resulting shape.

Please note I'm maintaining a fork of this project on my account, as I need the output in a different format

Rather than drawing orthogonal lines from the centre to measure the screen size, this uses a flood fill algorithm, and measures the size of the resulting shape.

Please note I'm maintaining a fork of this project on my account, as I need the output in a different format
@c0bra
Copy link
Owner

c0bra commented Jun 13, 2018

@jacobp100 Good stuff! I'm checking it out and it seems to work pretty well.

One thing I noticed is that the parse-frames.js function is significantly slower (which makes sense), however I can get a ~50% speedup by using the cluster module:

image

I'll work on opening a PR for that after I get yours fully tested and merged.

@c0bra c0bra merged commit 7cb4f16 into c0bra:master Jun 13, 2018
@c0bra
Copy link
Owner

c0bra commented Jun 13, 2018

Actually looks like part of it was either just load on my machine or I was potentially running under node 6. Node 8 looks like this:

image

Much better!

@jacobp100 jacobp100 deleted the patch-1 branch June 13, 2018 20:24
@jacobp100
Copy link
Contributor Author

That's huge! Looks like the improvements in node are really showing off!

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.

2 participants