fieldOfView / Cura-MaterialSettingsPlugin

A Cura plugin that lets the user select which print settings are available on the Materials pane of the preferences.
GNU Affero General Public License v3.0
13 stars 5 forks source link

[4.1] Wrong UI implied defaults in materials. The GUI defaults to fdmprinter.json (or printerdef.json) instead of the material XML mother file. #2

Open filipgoc opened 4 years ago

filipgoc commented 4 years ago

Hi! First, I love this plugin. I've been putting a ton of settings into my materials for ages (I know you feel the same way), so the chance to allow users in general to do the same is priceless.

Thank you so much for all your contributions.

Now the trouble

It's summed up by this image:

image

What seems to be happening is that Cura goes all the way back to fmprinter definition and pulls the values into the active printer stack and then takes that as the default instead of the material xml file.

This is, however, not just inconvenient, but potentially dangerous.

In the hands of unsuspecting user, who just wants to use the defaults, completely wrong settings sprint up (from mixing bowden vs direct drive retraction (current issue for many published printers in the stack) to completely wrong print temperatures for materials!

Solution?

  1. First thing would be probably to not allow editing of default materials and force users to duplicate and customize. Does not solve, but it's a part.
  2. The second thing would be to only show the 'reset' icon when the user themselves changes something against the root material xml file (the linked one, if it's a duplicate). This actually may need to be fixed in cura... but this plugin could maybe do it by looking into the xml and insert these into the stack as the de-facto defaults? Hm.

More info

I originally wrote this up for Cura repo before I realized it was an issue created by this plugin, so... it's very lengthy :-) You probably don't need to read it. But if you need the info, it's below.

Application version

4.1 I suspect this is not 4.1 issue, I just have not looked into this before.

Platform

Mac High Sierra

Printer

Tested with IMADE3D JellyBOX, Cartesio, and a few others. I randomly picked a few printers that I could easily trace through the files.

Reproduction steps

I'll use imade3d JellyBOX 2 and generic PLA an example, but I've done the same exploration with other printers as mentioned above. The issue is much broader than this. JellyBOX 2 is a simple use case because it uses separate generic PLA files.

  1. Add and activate imade3d JellyBOX 2
  2. Go to materials and select generic PLA
  3. You can see many 'reset to default' icons.
  4. Look at retraction_amount. It's 1.3
  5. Click on the 'reset to default' icon. It resets to 6.5 (Just an example, many other values suffer from this).
  6. The point is: the reset to default icons should not be here at all. This is an unmodified master material default profile that the user cannot modify - it should not be in the 'resettable' state.

Actual results

See above.

I looked over the stacks with God mode.

What seems to be happening is that Cura goes all the way back to fmprinter definition and pulls the values into the active printer stack and then takes that as the default instead of the material xml file.

Expected results

The defaults should be the material defaults, which of course get overridden with quality settings as necessary. But I understand there must be some fallback, and the printer def is just the beginning of the stack.

Therefore, the REAL issue is that these values should not be in the 'resettable' state.

These are unmodified master material default profiles - it should not be in the 'resettable' state until the users change something. (Arguable, the users should not even be allowed to modify these and be forced to make a duplicate instead, but that's another question.)

There may be lack of clarity of what variables materials should belong to materials (indeed there have been many discussions), but at least for the ones that ship with Cura the defaults should end there.

Additional information

I say a lot of 'should' in this post, but I'm open to various solutions. Or maybe the solutions lie with everyone defining their own defaults for everything (which I will probably do now), but that's a workaround. I'm not sure the UI exposure is philosophically right here. (The inheritance is already complicated enough, I probably would not touch that :-) )

The simplest example is retraction_amount as there are many direct drive machines in the current lineup that use around 1mm, but the fdmprinter is bowden style around 6mm.

fieldOfView commented 4 years ago

Thanks for the extensive report.

The "revert" buttons have a special function in the print settings tab: they remove the setting value from the XML file, thus having the value of the variant, definition changes or definition used and shown instead. This is by design. Perhaps I can change the icon and tooltip of the button to make this clearer.

You did highlight a bug; the generic materials are read-only, so it should not be possible to change them (nor remove values from the XML files, because these changes would not be kept). This has been fixed in d7427ff147ef82b109ae01ea9085f4c07a638fdf.

filipgoc commented 4 years ago

Yeah. It is by design. I do wonder whether the design is flawed here, but that's for Cura repo. If I feel strongly enough about this, I may re-post there. Thanks for the bug fix 👍

fieldOfView commented 4 years ago

The design you are opposing to is mine; Filing this against the Cura repo will do you no good.

I can remove the "revert" icons easily. But then you have no way to remove a setting from an XML file from the UI. How would you suggest I add that functionality, in a way that does not conflict with the rest of Cura?

A dialog that does not have a way to revert is the Machine Settings dialog. Once you change for example the start gcode field, there is no way to revert to default, without opening configuration files in a text editor. I wanted to prevent that in this dialog.

filipgoc commented 4 years ago

It's happy to see you're open to talk about the architecture.

The general trouble is of course what settings should override what under what conditions. As soon as we have more than one stack (materials/ hotends/ machines), the war arises. There is the general confusion that values can be defined in various places, but that's unavoidable. That's just life and inheritance.

Therefore, I don't think the 'revert' button, which is extremely useful, can blindly point all the way back in all cases. It may need to differentiate depending on where in the UI you are and what's defined where in your materials/machines/profiles.

  1. Simply put, I'm thinking that
    • the material GUI should allow me to re-create and revert back to the material XML without having to open a text editor ^[1]
    • the main UI window should allow me to revert back to the quality profiles without opening a text editor
    • the machine dialog should allow me to revert back to the machine json without having to open a text editor
  2. Since now all settings can be put in the material xml file (your work, too :-) ), treat any settings that is included in the material profile as the fallback default in the scope of the material dialog.
    • For example, if I have 95</cura:setting> in the material, then that should be the value the material reverts to (in the material dialog).
    • In addition, the revert button should only be visible if the value differs from this the cura:setting
    • The button should also not be visible if I unlink the material from the parent. (Presuming I duplicated some material to create my own).
    • Since I am in the material section of the UI, I don't think these values should revert beyond the scope of the material xml files -if the xml has the value defined.
    • IF the value is not defined in the material file, then the default should come from wherever it is defined. Just go down the stack :-), but stop as soon as possible (instead of going all the way down to fdmprinter.json or something).
  3. Indeed there's the inability to revert the machine settings. Why don't we just revert to the definition json, for gcode scripts namely? Do you know if there a reason or is it just deemed unimportant to implement?

^[1] footnote:

I should be able to trust the material dialog to not go beyond that scope of materials on the reverts - even if the dialog may display values from other places if they are not defined in the material xml.

If it proves too confusing, maybe at some point there could be more information given. Since this plugin allows display of all settings in the material dialog, maybe it could also indicate which ones are actually defined in the material and which ones come from elsewhere. I'm not sold it's needed.

fieldOfView commented 4 years ago

Long story short, I understand what you are saying, but you have a misconception about how files and the state in Cura work.

Files are loaded and parsed into the "state" in Cura. When a setting is changed or added, the state is marked "dirty" and after a short period of time the state is then written to disk. From then on the files reflect the state in Cura. So there is no way to revert to what is in the file, because the file reflects the state. Changing that is way beyond the scope of this plugin.

fieldOfView commented 4 years ago

maybe it could also indicate which ones are actually defined in the material and which ones come from elsewhere. I'm not sold it's needed.

This is something that I have wanted to tackle for a while, because the current situation is confusing.

Cura has a stacks of settings, typically the following "levels":

Each level in the stack can theoretically have a value for any Cura setting. Architecturally, there is no rule that says what setting goes in what level in the stack. For each setting, Cura traverses the stack from top to bottom, first looking for a value in the "user" level. If there is no value in the user level, the "quality changes" level is queried if it has a value for the setting, until eventually the default setting is retrieved from the printer definition (which has also contains all the defaults from fdmprinter.def.json).

What the revert-button does is that it removes the setting from the level (either from the "user" level in case of the sidebar, or from the "material" level in the Print Settings tab). With the value removed from the level, the value shown is going to be from one of the lower levels.

So now there is the inclarity you highlight; it is not clear what level the displayed value comes from. But more importantly, it is also not clear if the value is actually going to be used. A value in the "quality changes" level always trumps a value set in the "material" level, and the user does not see that.

filipgoc commented 4 years ago

The 'Bug' At Hand

What the revert-button does is that it removes the setting from the level (either from the "user" level in case of the sidebar, or from the "material" level in the Print Settings tab). With the value removed from the level, the value shown is going to be from one of the lower levels.

More general thoughts below.


The Issue At Large

Each level in the stack can theoretically have a value for any Cura setting. is also not clear if the value is actually going to be used. A value in the "quality changes" level always trumps a value set in the "material" level, and the user does not see that.

Yes. This is what the integral issue is.

Highlighting where the values come from is one possible step towards solution. Another part of this kind of solution would be highlighting where the values under the reset button come from. Like a preview of a link before you click on it. Hover over to see what value will be used in the revert (and from where.)

However, I also feel that while this kind of solution is absolutely marvelous for power users, it should be unnecessary. There should be some UI solution that behaves so predictably, that more information is not needed.

Invisible Solution?

When a setting is changed or added, the state is marked "dirty" and after a short period of time the state is then written to disk. From then on the files reflect the state in Cura...

Thanks for explaining this part to me. It makes a lot of sense.

Still, even if technologically 'reverting to a file' is impossible, would you agree the larger point of my argument? - That the values should reset to the values of the related 'files' depending on where you are in the UI?

I presume the current technological challenge is that Cura saves only the value of value in the current state but not where the values come from?

fieldOfView commented 4 years ago

Do you literary mean that the revert is designed to throw away whatever comes from the 'material' stack and therefore go down to a lower level to variant of definition?

Yes.

The desired defaults for materials are in the materials

As I have tried to explain, they are not.

It would make sense to enable throwing away something like material_user_changes.

Such a concept does not exist in Cura. This would be something to take up with the Cura team.

would you agree the larger point of my argument? - That the values should reset to the values of the related 'files' depending on where you are in the UI?

No, based on the current architecture of Cura, I do not agree.

I agree that when showing a value for a setting, it would be (much) better if

Currently, the "sidebar", the profile overview on the Profiles pane of the preferences, the Print settings tab on the Materials pane, and the settings in the Machine Settings all look and behave differently, but they all affect the settings stack in the same way without showing how they influence eachother. I agree that this is confusing.

fieldOfView commented 3 years ago

I am revisiting this in master.

fieldOfView commented 3 years ago

In the next version of the plugin, there will no longer be a set of settings that is made visible and either has a value (and a revert button) or shows the fdmprinter default. Instead, the list will show only those settings that actually have a value set in the xml file.

The interface will be consistent with eg the Per Model Settings plugin, so there will be a "Select settings" button below the list of settings by which you can select which settings are included in the xml file. Removing a setting from the list will remove it from the xml file (thus reverting to the default value set in the fdmprinter or printer definition json file).

image