deeice / lsynth

LDraw synthesis tool
9 stars 3 forks source link

Can't adjust vertical center of loop constraint #2

Closed elfprince13 closed 6 years ago

elfprince13 commented 6 years ago

I added the following:

0 // Plate 2 x 2 with Wheel Holder
1 7 0 -5 28 0 0 1 0 1 0 -1 0 0 4488.dat

to the BAND_CONSTRAINTS section of my lsynth.mpd, and found no matter what value I stick in the y-offset position (currently -5), the vertical positioning is not altered. I notice that while a small handful of other constraints have altered x or z offsets, none appear to have an altered y offset, so I don't have any further experimental controls to play with.

I'm wondering if there's any connection between this and #1

deeice commented 6 years ago

It's been like 10 years since I did much of anything with this code, so it could be quite a while before I figure anything out. It may simply be that the band synthesis is typically done all in one plane so the other dimension is ignored. But I couldn't say for sure right now. The comments around line 1206 of band.c do seem to suggest that an attempt to make use of all constraint offsets was abandoned for some reason.

Have you tried asking this question on the forums at ldraw.org? There may be people there with more recent lsynth experience, and perhaps a better answer.

--Don--


From: Thomas Dickerson notifications@github.com Sent: Monday, April 9, 2018 4:45 PM To: deeice/lsynth Cc: Subscribed Subject: [deeice/lsynth] Can't adjust vertical center of loop constraint (#2)

I added the following:

0 // Plate 2 x 2 with Wheel Holder 1 7 0 -5 28 0 0 1 0 1 0 -1 0 0 4488.dat

to the BAND_CONSTRAINTS section of my lsynth.mpd, and found no matter what value I stick in the y-offset position (currently -5), the vertical positioning is not altered. I notice that while a small handful of other constraints have altered x or z offsets, none appear to have an altered y offset, so I don't have any further experimental controls to play with.

I'm wondering if there's any connection between this and #1https://github.com/deeice/lsynth/issues/1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/deeice/lsynth/issues/2, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABfIZu-KqyXSwhG7n3WGqWHBM_KnY3SGks5tm5AMgaJpZM4TM3Ph.

elfprince13 commented 6 years ago

I haven't asked around, but Google didn't turn anything up either.

I noticed that there was a brand new GitHub repo and thought I'd bring it up here. In terms of making something work for my particular model, I can always add an extra layer of indirection by making a part that includes 4488.dat in the necessary orientation and using the new part in the lsynth.mpd config.

elfprince13 commented 6 years ago

Following up, making notes for myself to check later: The odd part is that the commented section you mentioned earlier, which is zeroing out constraint offsets, is disabled by preprocessor macros: https://github.com/deeice/lsynth/blob/888b5a45a705e8cd4e1de41f3229185b45377ff8/lsynth/lsynth/band.c#L1205-L1224

But it does give me a starting place to set some breakpoints and see what's going on.

deeice commented 6 years ago

Yes, the commenting out by preprocessor macros is something I tend to do when disabling code that I want to revisit someday. I might have moved the part offset adjustments earlier or later in the pipeline, but simplified for some reason that now eludes me. Maybe something to do with the band synthesis being planar?


From: Thomas Dickerson notifications@github.com Sent: Friday, April 13, 2018 2:58 PM To: deeice/lsynth Cc: deeice; Comment Subject: Re: [deeice/lsynth] Can't adjust vertical center of loop constraint (#2)

Following up, making notes for myself to check later: The odd part is that the commented section you mentioned earlier, which is zeroing out constraint offsets, is disabled by preprocessor macros: https://github.com/deeice/lsynth/blob/888b5a45a705e8cd4e1de41f3229185b45377ff8/lsynth/lsynth/band.c#L1205-L1224

[https://avatars0.githubusercontent.com/u/1558630?s=400&v=4]https://github.com/deeice/lsynth/blob/888b5a45a705e8cd4e1de41f3229185b45377ff8/lsynth/lsynth/band.c#L1205-L1224

deeice/lsynthhttps://github.com/deeice/lsynth/blob/888b5a45a705e8cd4e1de41f3229185b45377ff8/lsynth/lsynth/band.c#L1205-L1224 github.com lsynth - LDraw synthesis tool

But it does give me a starting place to set some breakpoints and see what's going on.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/deeice/lsynth/issues/2#issuecomment-381162805, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABfIZkRIGNj5uIBUZ28YBgEah8buoyvlks5toL0pgaJpZM4TM3Ph.

deeice commented 6 years ago

I took a longer look at that code and I now suspect the reason I didn't just add the offsets from the constraints there is because at that point everything has been rotated to get the first constraint from the model (constraint[0]) oriented in an imaginary XY plane. So the band_constraint offsets from lsynth.mpd would need to be rotated by whatever rotation was applied to get into that plane before adding them to the constraint parts used in the model. Or something like that. Also I don't see why constraint[0] is skipped in the loop, despite the comment in step 4 above, so that probably needs fixing too.

deeice commented 6 years ago

Ok. This seemed to fix it for me. https://github.com/deeice/lsynth/commit/8ea3a4c86d8e22165a6f709364fa8a96753fe856

Please test and verify if you get a chance.

elfprince13 commented 6 years ago

It appears to be working on my test model. Thanks for all the help digging into this.

elfprince13 commented 6 years ago

Oops, I take that back. It looks like the translation constraints aren't being rotated correctly.

In my earlier test, none of the constraints were rotated, and this was the result:

screen shot 2018-04-15 at 10 59 51 pm

In the newer test, they are rotated so the wheel-pins are on top, and one constraint appears correct but the other does not:

screen shot 2018-04-15 at 10 59 30 pm
elfprince13 commented 6 years ago

In terms of a MWE to replicate the working and non-working test-cases, this:

0 SYNTH BEGIN RUBBER_BAND 15
0 SYNTH SHOW
1 71 22.926786 40 -199.822845 0 -0.997564 -0.069756 -1 0 0 0 0.069756 -0.997564 4488.dat
1 71 -22.926786 40 -199.822845 0 0.997564 -0.069756 -1 0 0 0 0.069756 0.997564 4488.dat
0 SYNTH END

is the failing case. and this:

0 SYNTH BEGIN RUBBER_BAND 4
0 SYNTH SHOW
1 16 0 128 80 1 0 0 0 1 0 -0 0 1 4488.dat
1 16 0 176 130 1 0 0 0 1 0 -0 0 1 4488.dat
1 16 0 128 150 1 0 0 0 1 0 -0 0 1 4488.dat
0 SYNTH END

is the working case.

elfprince13 commented 6 years ago

Sorry for the stream of consciousness here in the comments -

My intuition for this is that somewhere you're transforming the translation separately from the rotation, and have a hand-coded matrix somewhere that's off by a sign error in some element.

In particular, if I rotate the misbehaving piece so that the wheel-post is down (180 about z, relative to it's orientation in the image above), then everything appears correct:

screen shot 2018-04-15 at 11 16 17 pm
deeice commented 6 years ago

Good find.

The code creates an imaginary third constraint (a copy of the first) to close the band, and there were some off by 1 loop counter errors in the code where it didn't rotate that constraint properly. Perhaps I missed one of those, or improperly "fixed" one. Hopefully that's the problem, but I wouldn't be surprised if it's what you said.

Unfortunately I don't have much time until next weekend to look at this. If you want to look at the code, there are some calls to showconstraints commented out in the band.c code that you can uncomment to see what happens to the constraints at various stages in different colors. That can help pinpoint which stage has the error.

deeice commented 6 years ago

rotband Needed to rotate the lsynth.mpd band offsets to the orientations of the modeled constraints before applying the offsets. https://github.com/deeice/lsynth/commit/b00a4f8d669be134e4a67291535d145142b45128

Another round of test and verify?

elfprince13 commented 6 years ago

I apologize if this nerd-sniped you too badly ;) Thanks again for the help!

Both the rotated and unrotated versions are looking good in my model now. I checked out the sample rubber band model from here as well.

I think we can safely mark this as closed.