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.

Sign up for GitHub

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

Jump to bottom

Deprecate TangentBinormalGenerator #2144

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

Deprecate TangentBinormalGenerator #2144

wants to merge 6 commits into from

Conversation

riccardobl
Copy link
Member

@riccardobl riccardobl commented Nov 9, 2023 •
edited
Loading

The purpose of this PR is to deprecate TangentBinormalGenerator

  • Deprecate TangentBinormalGenerator in favor of MikktspaceTangentGenerator
  • Add a bit more context to the comment in MikktspaceTangentGenerator
  • Remove the part about it being experimental, since at this point it has been tested and patched enought to be considered out of this stage
  • Add generate(Mesh) to match TangentBinormalGenerator
  • Move debugging utils from TangentBinormalGenerator into TangentUtils to be used with MikktspaceTangentGenerator
  • Replaces the usage of TangentBinormalGenerator in all the examples and tests (except the Parallel Tangent test since out mikktspace doesn't have a parallel implementation).

This PR needs to be tested

codex128
codex128 suggested changes Nov 14, 2023
View reviewed changes
Copy link
Contributor

@codex128 codex128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a typo on a javadoc link needs fixing.

jme3-core/src/main/java/com/jme3/util/TangentBinormalGenerator.java
@@ -59,9 +59,11 @@
import java.util.logging.Logger;

/**
*
* @deprecated This is an outdated and non-standard method. Please use @{link MikktspaceTangentGenerator}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change @{link to {@link.

@stephengold stephengold added this to the Future Release milestone Nov 14, 2023
@capdevon
Copy link
Contributor

capdevon commented Feb 6, 2024

Hi guys, what is the status of this PR ? Is it complete ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codex128 codex128 codex128 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Future Release
Development

Successfully merging this pull request may close these issues.
4 participants
@riccardobl @capdevon @codex128 @stephengold