Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CURA-12065] Fix (sort-of) conflict between per-object and per-mesh retraction. #2154

Draft
wants to merge 2 commits into
base: 5.9
Choose a base branch
from

Conversation

rburema
Copy link
Member

@rburema rburema commented Oct 23, 2024

We don't really have a set way to deal with settings that can both be per-extruder and per-mesh (sometimes called per-object in the engine) -- in general, building something like that would be a (new) feature (gap). This represents a workaround for a current issue where the overall settings are taken instead of the per-extruder ones, even though in the frontend, no per-mesh settings are overridden. Which happened because it always prefers the per-mesh settings over the per-extruder ones, and since the per-mesh settings where not overridden, it was just the 'top' ones. If you then overrode less then all of the extruders with a specific setting, it would default back to the 'top' settings (but if all extruders where overridden, that counted as not overridden again w.r.t per-extruder).

This commit 'fixes' that by assuming that if you have any setting different per extruder, you want that rather than the per mesh ones. This isn't necessarily the case (and also definitely not as granular as can be, it takes all retraction and wipe settings as one....) but it may do the job for now.

rburema and others added 2 commits October 23, 2024 15:49
…tings.

We don't really have a set way to deal with settings that can both be per-extruder and per-mesh (sometimes called per-object in the engine) -- in general, building something like that would be a (new) feature (gap). This repressents a workaround for a current issue where the overall settings are taken instead of the per-extruder ones, even though in the frontend, no per-mesh settings are overridden. Which happened because it always prefers the per-mesh settings over the per-extruder ones, and since the per-mesh settings where not overridden, it was just the 'top' ones. If you then overrode less then all of the extruders with a specific setting, it would default back to the 'top' settings (but if all extruders where overridden, that counted as not overridden again w.r.t per-extruder).

This commit 'fixes' that by assuming that if you have any setting different per extruder, you want that rather than the per mesh ones. This isn't necesarily the case (and also definitely not as granular as can be, it takes all retraction and wipe settings as one....) but it may do the job for now.

CURA-12065
Copy link
Contributor

@wawanbreton wawanbreton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This really seems hacky to me 😬 I have the feeling that it just moves the issue to an other place. Does it actually fix some cases ? If yes, then I would be ok to integrate it, but with a follow-up ticket to do a bigger refactoring.

@rburema
Copy link
Member Author

rburema commented Oct 24, 2024

Does it actually fix some cases?

It does fix the example case in the jira-ticket. I'm not sure if that's from an actual user though, or if QA made it.

@rburema
Copy link
Member Author

rburema commented Oct 24, 2024

Mothballed.

@rburema rburema marked this pull request as draft October 24, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants