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

artoolkit border labeling mode implementation #711

Closed
wants to merge 1 commit into from
Closed

artoolkit border labeling mode implementation #711

wants to merge 1 commit into from

Conversation

umutto
Copy link
Contributor

@umutto umutto commented Jan 25, 2020

⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️

What kind of change does this PR introduce?
New feature

Can it be referenced to an Issue? If so what is the issue # ?
#599

How can we test it?
I've created a simple aframe example under aframe/examples/white-region-marker.html. Different markers can be created and tested using this tool (make sure to select markers have white borders option).

Summary
Connects the parameter settings for underlying ARToolKit's border labeling modes. (AR_LABELING_BLACK_REGION and AR_LABELING_WHITE_REGION). Border labeling mode can be set from scene in the same way as detection mode or matrix code types.

Markers with black borders does not perform well (or at all without a white buffer zone) when they are put over a black background like a dark desk. Markers with white borders (which are literally inverted markers) solve that issue (but similarly they don't perform well on white desks).

I have been regularly using these markers for the last year with an old modified version of AR.js, I've just seen the issue and wanted to contribute. So I've tested with many different devices.

Does this PR introduce a breaking change?

No, it is at the very surface, superficial change. Only code that have been implemented are assertions and parameter bindigns. Underlying ARToolkit connection and the code to change parameters was already written, just not connected to the aframe and three.js endpoints.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser

OS: Windows 10
Browser: Firefox, Google Chrome

OS: Android 10
Browser: Chrome for Android

Other information

@kalwalt
Copy link
Contributor

kalwalt commented Jan 25, 2020

Hi @umutto thank you for this PR! I will try it if i find a bit of time. 😄

@nicolocarpignoli
Copy link
Collaborator

can you please re open this one to the new 'dev' branch? thanks

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.

3 participants