CURA-13250 Define settings for chamfered support base#21714
CURA-13250 Define settings for chamfered support base#21714Erwan MATHIEU (wawanbreton) wants to merge 2 commits into
Conversation
Test Results24 950 tests 24 948 ✅ 51s ⏱️ Results for commit cd7e52a. ♻️ This comment has been updated with latest results. |
| } | ||
| } | ||
| }, | ||
| "support_base_inside_width": |
There was a problem hiding this comment.
Why do base the "Support Base Height" settings and "Support Base Slope" settings have parent, children but the "Support Base Width" settings don't have a parent?
There was a problem hiding this comment.
And what is the difference between this setting and "Enable Inner Support Base"?
There was a problem hiding this comment.
Why do base the "Support Base Height" settings and "Support Base Slope" settings have parent, children but the "Support Base Width" settings don't have a parent?
For the height and the slope, it does make sense to have the same value for the inner and outer bases, so you just set the value of the parent, and it sets the value of both the children.
For the width, they will almost never have the same value, since they depend on the inner brim width and global brim width, so having a parent value doesn't make sense here.
And what is the difference between this setting and "Enable Inner Support Base"?
In practice, you can indeed disable the inner base by settings a 0 width (or height), however for the retro-compatibility I needed to keep the "Enable Outer Support Base" anyway, so for the sake of consistency it seemed logical to also have a "Enable Inner Support Base". I do think it makes some sense as a user, that the whole feature can be enabled/disabled at once.
There was a problem hiding this comment.
For the width, they will almost never have the same value, since they depend on the inner brim width and global brim width, so having a parent value doesn't make sense here.
We can still have child values that differ from it's parent or siblings, see default profile for s3 for example.
Benefit of adding this parent child relationship is that it nicely groups the values in the settings.
There was a problem hiding this comment.
This example is unrelated, the top_bottom_thickness and wall_thickness do not share a parenting relationship, at best a formula that makes one depend on the other.
I do remember a discussion with Jaime about the parenting. We had an external PR that added a lot of parenting for grouping purposes. Although looking good in the UI, it was rejected because from what I understood, parenting is really only for a setting that can have multiple more-defined values of itself. For instance, wall_extruder_nr is a parent of wall_0_extruder_nr and wall_x_extruder_nr. The idea is that you can set them both at once by using the parent, or use different values with the children, in which case the parent becomes useless (and is actually greyed out).
So if we had a Support Base Width with Support Inner Base Width and Support Outer Base Width as children, what would be the value of the Support Base Width ? The innner/outer also don't have the same limits, so they are impossible to combine into a parent.
There was a problem hiding this comment.
I do remember a discussion with Jaime about the parenting. We
When adding a parent<->child setting, then the parent setting should never be used by the engine and the child setting should dictate how the setting is used. They can very well have different limits etc.
There was a problem hiding this comment.
Then could you make a proposal for the parent definition ? I fail to see how this can work 🤔
2 participants
This PRs adds a few settings to control the new support (tree and normal) base, which can be enabled independantly inside and/or outside the actual support area. The base makes the bottom of the support much sturdier and also improves adhesion to the buildplate, so it is now possible to use higher supports, or tree supports with less trunks.
In order to ensure retro-compatility and consistency, the
support_brim_enablesetting is now actually the "Support Inner Base" and a newsupport_outer_brim_enableis added for the "Support Outer Base".Also to note: both the inner and outer base outside widths are capped to their corresponding brim widths. Thus when using a brim, the user should set the brim width first, then the base width. When no brim is used, the values can be set without this restriction.
CURA-13250
Requires Ultimaker/CuraEngine#2346