search

StandardCyborg / StandardCyborgCocoa

Everything you need for 3D scanning on iOS
https://www.standardcyborg.com
Other
143 stars 49 forks source link

StandardCyborgUI: Add `onRenderedPointCloudImageReady` hook in PointCloudPreviewViewController #3

Closed irtemed88 closed 4 years ago

irtemed88 commented 4 years ago

Please review after https://github.com/StandardCyborg/StandardCyborgCocoa/pull/2.

This hook will allow consumers of PointCloudPreviewViewController to reliably access renderedPointCloudImage since that prop is set in viewDidAppear.

This was an issue for the case where the view controller was added as a child view controller.

irtemed88 commented 4 years ago

Yeah I actually had it that way first. The issue I ran into was if a consumer sets the closure after the image is available do we have a responsibility to call the closure immediately or should the consumer be responsible for checking the prop before setting the closure? It's not a big deal to me either way.

irtemed88 commented 4 years ago

... as for making this a notification, I'm not sure it's worth the hassle? If we end up needing it in a bunch of other places I think we can add that in but for now I don't foresee many use cases where it's super beneficial.

aaptho commented 4 years ago

Yeah I actually had it that way first. The issue I ran into was if a consumer sets the closure after the image is available do we have a responsibility to call the closure immediately or should the consumer be responsible for checking the prop before setting the closure? It's not a big deal to me either way.

I don't think the consumer should have any expectation of it being called if the image was already available.

irtemed88 commented 4 years ago

Ok! Updated the name of the closure to better reflect it's purpose and also updated the title of the PR.