fix mmd ikbone bug, mmd parser #21249

ingbunga · 2021-02-10T13:59:59Z

Related issue: --

Description

There are two problems.

Shift jis to utf table isn't work with some model. ( mmdparser.module.js )

I use Textdecoder instead of shift jis to utf table.
bone that relative with IKbone go to the wrong place. ( MMDAnimationHelper.js )
before:

after fix GrantSolver:

I just fixed a simple mistake.

takahirox · 2021-02-10T14:54:43Z

Thanks for the bug report.

The files under examples/js(m)/libs are from external sites. You need to send issues/PRs for them to the original authors' repository.

In this case, mmd-parser.module.js is from https://github.com/takahirox/mmd-parser. And the portion you edited in mmd-parser.module.js is from charset-encoder-js https://github.com/takahirox/charset-encoder-js. The both are my projects.

Regarding TextDecoder stuff, if I remember correctly there is a reason why I made my own text encoder/decoder, not use TextDecoder but I don't remember the exact reason... Give me time to remember it... Update: According to my tweet Chrome did not support TextDecoder when I wrote MMDLoader? If it's true, it may be time to switch to TextDecoder.

And for both 1 and 2 problems, would you mind if sharing the model files with me? I would like to investigate the root issues.

mrdoob · 2021-02-10T15:06:25Z

Regarding TextDecoder stuff, if I remember correctly there is a reason why I made my own text encoder/decoder, not use TextDecoder but I don't remember the exact reason... Give me time to remember it...

I suspect it was because not many browsers supporter TextDecoder at the time.

takahirox · 2021-02-10T15:11:43Z

I suspect it was because not many browsers supporter TextDecoder at the time.

Yeah, as I updated the comment, it seems true. MMDLoader is getting on years...

ingbunga · 2021-02-10T15:15:22Z

Thanks for the bug report.

The files under examples/js(m)/libs are from external sites. You need to send issues/PRs for them to the original authors' repository.

In this case, mmd-parser.module.js is from https://github.com/takahirox/mmd-parser. And the portion you edited in mmd-parser.module.js is from charset-encoder https://github.com/takahirox/charset-encoder-js. The both are my projects.

Regarding TextDecoder stuff, if I remember correctly there is a reason why I made my own text encoder/decoder, not use TextDecoder but I don't remember the exact reason... Give me time to remember it...

And for both 1 and 2 problems, would you mind if sharing the model files with me? I would like to investigate the root issues.

First of all,

I used same model in both problem.

https://bowlroll.net/file/155105
here are link...

And,

First problem is char code not in table.

And second problem is after I use CCIDKSolver,

in my case,

model's leg is moving with target.

ex)
before i move leg

after i move leg

Sorry for my bad English. thanks.

takahirox · 2021-02-10T15:24:15Z

Regarding the problem 1, probably I remembered the reason and it looks the time to switch to TextDecoder/Encoder. I would like to fix it in mmd-parser-js https://github.com/takahirox/mmd-parser or charset-encoder-js https://github.com/takahirox/charset-encoder-js repos and then update the one in Three.js repo. PR is welcome to my repos. And would you please remove the change for mmd-parser.module.js from this PR so far?

Regarding the problem 2. Thanks for sharing the model. Let me test with it to figure out the root issue. Note that I'm hospitalized now so it may take time.

Sorry for my bad English. thanks.

No worry! I know mine is bad!

ingbunga · 2021-02-10T15:43:07Z

@takahirox
I've sent pull request of version of TextDecoder version on your repository.
Can you allow it?
And I undid change of mmd-parser-js.

takahirox · 2021-02-10T15:46:56Z

@ingbunga Thanks. I will review there hopefully soon.

mrdoob · 2021-02-20T15:21:47Z

@takahirox The diff is now very tiny.

takahirox · 2021-02-21T01:25:12Z

A bit confusing... Which is this change for, optimization or fixing ik?

ingbunga · 2021-02-21T13:35:45Z

A bit confusing... Which is this change for, optimization or fixing ik?

The first commit was fixed the ikbone bug.
But performance was horrible.
So I found detail reason for bug and rewrite code.
After rewrite Ikbone still works well.

In nutshell, it can be both of all

takahirox · 2021-02-23T01:53:40Z

Hi, thanks for the explanation. But to be honest I don't think ignoring negative ratio is a true solution because authors can set negative ratio in MMD.

I speculate the root issue is this line

quaternion.slerp( parentBone.quaternion, grant.ratio );

because the second argument of Quaternion.slerp() accepts [0, 1] so we may need to lerp differently for negative ratios.

ingbunga · 2021-02-23T04:33:51Z

I agree.
My test was wrong.
I found what's happen if i set negative grant

3.5times faster than first commit

almost same performance with first version!

i misunderstood about it.

ingbunga · 2021-02-25T16:13:47Z

@takahirox

Hello,

I had many test and commit.

I think it's works well.

Can you review it?

negative grant works.

ik bone dosen't make bug.

grant + bone rotate works.

this is performance of how long taking one frame.

( ms )

performance before PR:
6.998884999984876
7.0240449999226255
7.0184440002136395
7.158275499625597

performance after latest commit:
6.984816999989562
6.984816999989562
6.955773499503266
6.673954500007676

how I performance Test

var sum = 0;
var count = 0;

function animate() {

    requestAnimationFrame(animate);

    var start = performance.now();
    helper.update( clock.getDelta() );
    sum += performance.now() - start;
    count ++;

    if( count === 10000 ) {

        console.log( sum / 10000 );

    }
    
    renderer.render( scene, camera );
    controls.update();

};

takahirox · 2021-02-26T01:16:12Z

Thanks for the updating and notification. I'm checking the PMX animation system specification and thinking how the right and best solution should be. The PMX animation system is a bit too complex so give me more time to review...

takahirox · 2021-02-27T07:32:41Z

@ingbunga

Would you mind if testing this branch 24eb8b6 and checking the problems can be resolved on your tests? I'm seeking simpler change for better PMX animation system simulation based on your investigation. Please refer to the inline comments for the change.

ingbunga · 2021-02-27T14:45:31Z

@takahirox

This is screen shot of 24eb8b6

I kinda bug isn't fixed.
I think you should solve grant once again after IK solved

Actually IK solved well, result is good.
But leg isn't move to solved location. ( grant not solved. )

And you didn't fixed code on MMDHelper.pose()
I think it's better to change GrantSolver.update(); ( for reduce confuse )

How about divide bone to IK related bone and normal bone, and solve grant IK related bone first and solve normal bone last ( after solve IK )?
I kinda that's working great!

In test, I only change s2uTable to TextDecoder because if I don't change code, it makes error while loading files

takahirox · 2021-02-27T23:22:48Z

@ingbunga

Thanks for the test. Can you share your tests online? I want to locally test more deeply.

I think it's better to change GrantSolver.update(); ( for reduce confuse )

TBH, I don't think it's a good idea because I don't want GrantSolver to have dependency with CCDIKSolver for the simplicity. I think this problem should be fixed in MMDAnimationHelper.

How about divide bone to IK related bone and normal bone, and solve grant IK related bone first and solve normal bone last ( after solve IK )?

Would you elaborate more for me why it would resolve the problem?

In test, I only change s2uTable to TextDecoder because if I don't change code, it makes error while loading files

What's platform do you use? I couldn't reproduce the error although I loaded the models you pointed out.

ingbunga · 2021-02-28T05:30:28Z

TBH, I don't think it's a good idea because I don't want GrantSolver to have dependency with CCDIKSolver for the simplicity. I think this problem should be fixed in MMDAnimationHelper.

OK! I understand.

What's platform do you use? I couldn't reproduce the error although I loaded the models you pointed out.

Thanks for the test. Can you share your tests online? I want to locally test more deeply.

I think error from VMD or VPD. I uploaded all of folder.
https://github.com/ingbunga/mmd_bug_happen_project

Would you elaborate more for me why it would resolve the problem?

OK! First of all, you should focus on bone 88. ( grant[4] )
This is bone that connecting edge of leg and body.

It's parent bone is bone 5.
So, this grant -1 works simple.

Out of this bone, when the mmd model turn its body,leg turn too.
Grant -1 prevent that.

with grant -1.

without grant -1.

But bone 88 is parent of bone 89 ( leg ik bone. )
So when we rotate bone 88, the ik is go wrong place.

like this photo that i uploaded.

In this case, in my idea is

solve iks
solve ik related bones.
solve iks again
In this time, we have solved IK and granted IK related bones.
So IK related bones dosen't need rotate,
and solved IK consider granted IK related bones.
solve normal bones.
And just move to solved place.

In my test, this worked.

takahirox · 2021-03-02T11:36:22Z

@ingbunga

Thanks for the explanation. I confirmed the commit still doesn't resolve the problems in your tests.

I made a new commit to follow the PMX Animation system more properly. I confirmed it resolves the problems. If you have other tests, models, or animations, would you mind if trying to test again? (I added a new MMDAnimationHelper constructor parameter pmxAnimation. Set true in your tests.)

takahirox@30e07fa

Thanks for sharing your solution idea. I would like to start with simulating PMX animation system as much as possible, and then I would like to think of optimization like your idea.

ingbunga · 2021-03-02T13:52:24Z

@takahirox

All bones working well,
Performance is good!

Avg 1 frame ms:
6.973566999881587
7.083169999913662
7.051487999952224
6.967415999931109

I keep on testing,
But seem all thing is right!

Should I close this PR?

takahirox · 2021-03-03T02:32:48Z

@ingbunga

Thanks for testing.

Should I close this PR?

Yeah... I wanted to respect your PR but the change of the solution ended up being much bigger than I thought so it would be better to close this PR and I make a new PR, sorry.

Instead I would like to ask the Change log editor (@Mugen87 ?) for adding your name next to me in r127 Change log.

ingbunga · 2021-03-03T04:58:18Z

Thank you so much!

Mugen87 · 2021-03-03T08:28:16Z

Instead I would like to ask the Change log editor (@Mugen87 ?) for adding your name next to me in r127 Change log.

Noted!

fix bugs

cd0e82d

Mugen87 requested a review from takahirox February 10, 2021 14:20

mrdoob added this to the r126 milestone Feb 10, 2021

undo

9a1f937

performance optimize

75cb8aa

mrdoob modified the milestones: r126, rXXX Feb 23, 2021

ingbunga added 3 commits February 24, 2021 03:14

Undid and optimize

2c89327

3.5times faster than first commit

logic change ( O(1) )

b75854d

almost same performance with first version!

Fix about grant.

d99268f

i misunderstood about it.

ingbunga closed this Mar 3, 2021

takahirox mentioned this pull request Mar 3, 2021

MMDLoader: Improve Animation system for PMX #21395

Merged

mrdoob removed this from the rXXX milestone Mar 3, 2021

takahirox mentioned this pull request Mar 3, 2021

Bump MMDParser lib to the latest one #21397

Merged

Uh oh!

fix mmd ikbone bug, mmd parser #21249

fix mmd ikbone bug, mmd parser #21249

Uh oh!

Conversation

ingbunga commented Feb 10, 2021

Uh oh!

takahirox commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrdoob commented Feb 10, 2021

Uh oh!

takahirox commented Feb 10, 2021

Uh oh!

ingbunga commented Feb 10, 2021

Uh oh!

takahirox commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ingbunga commented Feb 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Feb 10, 2021

Uh oh!

mrdoob commented Feb 20, 2021

Uh oh!

takahirox commented Feb 21, 2021

Uh oh!

ingbunga commented Feb 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Feb 23, 2021

Uh oh!

ingbunga commented Feb 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ingbunga commented Feb 25, 2021

this is performance of how long taking one frame.

how I performance Test

Uh oh!

takahirox commented Feb 26, 2021

Uh oh!

takahirox commented Feb 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ingbunga commented Feb 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Feb 27, 2021

Uh oh!

ingbunga commented Feb 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ingbunga commented Mar 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

takahirox commented Mar 3, 2021

Uh oh!

ingbunga commented Mar 3, 2021

Uh oh!

Mugen87 commented Mar 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

takahirox commented Feb 10, 2021 •

edited

Loading

takahirox commented Feb 10, 2021 •

edited

Loading

ingbunga commented Feb 10, 2021 •

edited

Loading

ingbunga commented Feb 21, 2021 •

edited

Loading

ingbunga commented Feb 23, 2021 •

edited

Loading

takahirox commented Feb 27, 2021 •

edited

Loading

ingbunga commented Feb 27, 2021 •

edited

Loading

ingbunga commented Feb 28, 2021 •

edited

Loading

takahirox commented Mar 2, 2021 •

edited

Loading

ingbunga commented Mar 2, 2021 •

edited

Loading