makehumancommunity / makehuman

This is the main repository for the MakeHuman application as such.
http://www.makehumancommunity.org
Other
1.18k stars 244 forks source link

Enables the 'edit' textbox for all Sliders #201

Closed fjardon closed 1 year ago

fjardon commented 1 year ago

Problem: We want to copy/paste a slider's value to another. For instance we want to copy the left-arm's lowerarm-muscles to the right-arm's lowerarm-muscles.

Solution: Enables the textbox value editor for all Sliders. We do this by setting a no-op value converter if none is specified. The no-op value converter simply copy the input to its output. We also fix small bugs, like sending 'onChanging' events when 'setValue()' is called. If not, derived classes like ModifierSlider won't update correctly.

joepal1976 commented 1 year ago

For reference, this is how the UI looks with the changes applied:

image

I am cautiously positive to the change, and do in principle think it is a good idea. However, it is visibly also a rather large change in something which is planned to be a patch release.

I'm going to request review by some other devs before merging this.

If it was made optional, with for example a checkbox under settings for enabling it, then I would not hesitate to merge it.

fjardon commented 1 year ago

Hi, thank you for the feedback. This is indeed a rather large change in the UI. I used Reallusion's "character creator" and it provides a similar feature: `Reallusion Character Creator Morph Tab`.

I will look in the source code to find how to control the feature with a setting.

fjardon commented 1 year ago

I updated the pull-request.

black-punkduck commented 1 year ago

hi there ...

all okay from my side ... a few lines in qtgui.py. imho not a big change. We had these things partly in the menu in modelling/measure, but with an additional unit. I tried a similar thing at home long time ago. I would appreciate this change.

Hmm, is there an official method to review?

greetings ... punkduck

Aranuvir commented 1 year ago

All in all I'm absolutely positive, too. There have been many requests for this feature in the past and I like the optional approach via settings. The only downside I see are upcoming endless discussions because these values are not from the real world.

Finally I was wondering if a 3 digit precision would add a bit more fine control to the modeling. And there is a small glitch with not updating the status bar values.

@ Joel: We could also bump the version minor number with this feature. But in that case I would also like to address the endless unicode issues before we release. Maybe we should have an internal discussion about the roadmap.

fjardon commented 1 year ago

Hi, thank you for the feedback.

If I remember correctly Reallusion's Character Creator uses the same -1 to +1 range without units for the morphs sliders.

For the precision of the edit box, it could be configurable in the settings too. But maybe that should be another commit.

I looked at the code and I found out that uncommenting the following line would fix the status bar glitch. Unfortunately, I wasn't able to understand why the line is commented in the first place:

diff --git a/makehuman/lib/modifierslider.py b/makehuman/lib/modifierslider.py
index ffbde174..7dbefab8 100644
--- a/makehuman/lib/modifierslider.py
+++ b/makehuman/lib/modifierslider.py
@@ -138,8 +138,7 @@ class ModifierSlider(gui.Slider):

     def onChange(self, value):
-        #G.app.callAsync(self._onChange)
-        pass
+        G.app.callAsync(self._onChange)

The first trace of this line is in commit: f6b431ac, that is the very first commit of the repository...

Is there any web page where I could see the roadmap of the project ? I have potentially other commits coming and maybe I should wait for another time before sending my pull-requests.

rwbaer commented 1 year ago

As I recall some slides use a 0 to 1 scale while others us a -1 to 1 scale. Using relative slider units would be a start before getting into translating to "measurement units" that could start endless discussions. The idea does seem to benefit artists even if done in slider units.

Aranuvir commented 1 year ago

@fjardon: Currently we don't have a real tool for roadmaps. I think we will have an internal discussion and we will open an extra issue on github summing up all important issues to be solved and features to be included before the next release.

Honestly I think now is the time to include new features and I'm looking forward for more pull requests from your side.

@joepal1976: I'd propose to merge. Maybe in a separate branch at first, if you think it's necessary.

joepal1976 commented 1 year ago

I'll merge and see if anyone has any opinions after testing nightly build.

rwbaer commented 1 year ago

Perhaps I'm ahead of the push, but I do not see the changes on today's nightly build 20220719 alpha (HEAD:f8ff3bc5) build.