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

Refactor object reload to handle 3mf files #16610

Merged
merged 3 commits into from
Nov 1, 2023

Conversation

pietchaki
Copy link
Contributor

@pietchaki pietchaki commented Aug 29, 2023

Description

3mf Files can have multiple models in them, and they can be named individually, but Cura was renaming all these models with its filename, not really an issue by itself but having the original names is better when there are many models in the file.
Also, when the file has multiple models, and user reloads them (right click -> Reload All Models), all objects from that file where being replaced by the first model in the file.
Another minor issue was that when reloading, Cura would read the same file multiple times, one for each model.

! Important: This should be accompanied by Ultimaker/Uranium#898, so that both model reload methods have the same behaviour

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Confirmed other filetypes where not affected by the refactor:

  • Opened a OBJ file and reloaded.

  • Opened STL file and reloaded.

  • Opened gcode file, and reloaded.

  • Loaded a 3MF file into Cura, verified the name of the models in the app are the model name in the file.

  • Right click the window and selected "Reload All Models", verified each model get replaced as expected.

  • Updated the 3MF file removing one of the models, ignored the "File has been modified" message{1}, right click->"Reload All Models". Confirmed it works as before and keep the mesh in the scene.

  • Updated the 3MF file, adding a new mesh into it, ignored the "File has been modified" message{1} and reloaded the models via right click. Confirmed it works as before and the new mesh is NOT loaded in the scene{2}.

{1}: The message about the file being modified and asking to reload is handles by Uranium, and not in Cura. Improvements to that process will be committed to Uranium.
{2}: This should be an improvement, as the newly added model should be loaded when we reload a file. This however should be accompanied with an update to the process described in {1} so that both have the same behaviour.

Test Configuration:

  • Operating System: Windows 10

Checklist:

Refactor object reload to better handling of 3mf files

Make a single file read for all objects from that file, and then
replace the objects in scene to the new ones, matching by name.

This is also required for an Uranium update to handle the automatic
file reload when a file change is detected.

3mf Files can have multiple objects in them. When that is the case,
and the file is updated, all objects from that file where being
replaced by the first object in the file.
@saumyaj3
Copy link
Contributor

Hey @pietchaki,

Thanks for your contribution 👍
I was just reviewing the PR, and came across a model where name was not replaced with "object id" and took the file name still. Can you maybe take a look again at the PR?
bug_array.zip

model_names

@saumyaj3 saumyaj3 marked this pull request as draft October 11, 2023 11:59
@pietchaki
Copy link
Contributor Author

@saumyaj3 Thanks for the report, but this file doesn't have name metadata for the objects, and for this reason we name them with the file name. That part is working as intended.

@pietchaki pietchaki marked this pull request as ready for review October 13, 2023 02:41
@saumyaj3 saumyaj3 merged commit 3121766 into Ultimaker:main Nov 1, 2023
@saumyaj3
Copy link
Contributor

saumyaj3 commented Nov 1, 2023

Hey @pietchaki Thank you for your contribution and for your work to make Cura better . 👯 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Community Contribution 👑 Community Contribution PR's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants