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

docs: Fix links, add jS API documentation #1434

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

djthegr8
Copy link

The links for control_utils, drawing_utils and camera_utils are incorrect, and all lead to the pose npm site.
Have fixed this, please review and merge

the links for control_utils, drawing_utils and camera_utils were incorrect
@djthegr8 djthegr8 changed the title Fix links in javascript.md Fix links, add jS API documentation Dec 25, 2020
@djthegr8
Copy link
Author

Have added documentation (to my knowledge) about the JavaScript API.
Please review and merge, as this is required by many.

@djthegr8 djthegr8 changed the title Fix links, add jS API documentation docs: Fix links, add jS API documentation Dec 25, 2020
@Choons
Copy link

Choons commented Dec 26, 2020

@djthegr8 definitely helpful! I am wondering what is/isn't exposed regarding access to the 3D coordinates of the mesh as well as the transformation matrix? It appears some things may be in Web Assembly modules in which case we need the names of the javascript function calls into them. it would be helpful to get a clear statement of what is available to us and what isn't (if any) in the javascript solution.

@djthegr8
Copy link
Author

djthegr8 commented Dec 26, 2020 via email

@Choons
Copy link

Choons commented Dec 26, 2020

Indeed. It's very strange that they would release a javascript solution so bare bones compared to the write up for the new Face Mesh touting the Geometry module, true 3D, camera position, etc. and then just refuse to show the uncompiled javascript source code. Thanks for making the effort-- perhaps it will spur the devs into giving us more info. It would be a lot easier on them than people posting hundreds and hundreds of questions in the issues section!

@djthegr8
Copy link
Author

djthegr8 commented Dec 26, 2020 via email

@djthegr8
Copy link
Author

@tyrmullen could you review?

@maciekmp
Copy link

Like it, also waiting for docs of pose package.

@djthegr8
Copy link
Author

I am honestly not too familiar with the pose package, but if anyone wants to send up a pr to my fork, you're welcome to do so

@maciekmp
Copy link

I was not saying is a job for you. It is just a shame that there is a set of great libs but without proper docs. From yesterday I am getting error like you can not send data before initialization. With just minified code it is not easy to find solution.

@djthegr8
Copy link
Author

I couldn't agree more with you. This is more of a case in smaller libraries, but one by Google surely shouldn't be like this.
I think the only way to force them to do this is send PRs like this one, by someone who understands (at least to an extent) minified code (like I did). 👍

@djthegr8 djthegr8 mentioned this pull request Mar 29, 2021
@mhays-google
Copy link
Contributor

I seem to be getting triaged bugs assigned to me now, so I'll be paying more attention to issues like this.

These doc changes are great, and all of this will be absorved upstream for our mid-April GitHubrelease.

As for documentation, check out #1408, in the short term, I've released the index.d.ts file for each package into the manifest so that you don't have to muck around in minified code just to figure out an interface.

I will write up a README.md here in the next few days for each package and stage it into NPM. I'm going to probably leave this bug open until we release in case there are some other nits surrounding this that I need to address.

Thank you everyone for the patience.

@djthegr8
Copy link
Author

djthegr8 commented Mar 31, 2021

@mhays-google the TypeScript files are pretty much excellent to decipher the code, thank you so much for releasing that.


Welcoming PRs to this branch btw, if someone finds inaccuracies or something after reading the comments, I personally do not have much time at the moment, so will be more than willing to merge

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

5 participants