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

[Bug] Face color of PartDesign Body not exported to STEP #6451

Closed
2 tasks done
Roy-043 opened this issue Feb 25, 2022 · 15 comments
Closed
2 tasks done

[Bug] Face color of PartDesign Body not exported to STEP #6451

Roy-043 opened this issue Feb 25, 2022 · 15 comments
Labels
Bug This issue or PR is related to a bug Color Regarding the color handling UI/UX WB Part Design Related to the Part Design Workbench
Milestone

Comments

@Roy-043
Copy link
Contributor

Roy-043 commented Feb 25, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Forums discussion

https://forum.freecadweb.org/viewtopic.php?f=3&t=66604

Version

0.20 (Development)

Full version info

OS: Windows 8.1 (6.3)
Word size of FreeCAD: 64-bit
Version: 0.20.27518 (Git)
Build type: Release
Python version: 3.8.12
Qt version: 5.12.9
Coin version: 4.0.0
OCC version: 7.5.3
Locale: Dutch/Netherlands (nl_NL)

Subproject(s) affected?

Core

Issue description

You can apply a face color to a feature in a PartDesign Body ("Set colors..." context menu option). But this color is not exported to STEP.

Anything else?

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@Roy-043 Roy-043 added the Bug This issue or PR is related to a bug label Feb 25, 2022
@luzpaz luzpaz added the WB Part Design Related to the Part Design Workbench label Feb 26, 2022
@tormyvancool
Copy link

Got same issue.

@luzpaz
Copy link
Contributor

luzpaz commented Feb 26, 2022

@tormyvancool this tells us nothing without your full About info

@tormyvancool
Copy link

@luzpaz sorry, please here my about:

OS: Windows 10 Version 2009 <<<< not clue why it tells WIndows 10 since it is Windows 11; but ok perhaps it's fine so.
Word size of OS: 64-bit
Word size of FreeCAD: 64-bit
Version: 0.19.24267 +99 (Git)
Build type: Release
Branch: Branch_0.19.3
Hash: 6530e36
Python version: 3.8.6+
Qt version: 5.15.2
Coin version: 4.0.1
OCC version: 7.5.3
Locale: English/United States (en_US)

@luzpaz
Copy link
Contributor

luzpaz commented May 16, 2022

@donovaly we should probably try to get this fixed before v0.20 release as well.

@luzpaz luzpaz added UI/UX Color Regarding the color handling labels May 16, 2022
@donovaly donovaly added this to the 0.20 milestone May 17, 2022
@donovaly
Copy link
Member

@yorikvanhavre maybe you have time to have a look?

@yorikvanhavre
Copy link
Member

Looking at it now, the culprit is basically here:
https://github.com/FreeCAD/FreeCAD/blob/master/src/Mod/Import/Gui/AppImportGuiPy.cpp#L364
this findColors() function only considers Parts. We should extend it to also consider other view providers which also have a DiffuseColor property (I suppose the Body does its job properly to populate the DiffuseColor). Looking into it now...

@yorikvanhavre
Copy link
Member

yorikvanhavre commented May 17, 2022

There is something I don't understand here: PartDesignGui::VierwProviderBody is derived from PartGui::ViewProviderPart, which itself is derived from PartGui::ViewProviderPartExt. So in the above code, if (vp && vp->isDerivedFrom(PartGui::ViewProviderPartExt::getClassTypeId())) should work for bodies too...
Maybe then the problem is that the body doesn't properly populate its DiffuseColor. I'll do some more testing

@yorikvanhavre
Copy link
Member

Ok indeed the body's DiffuseColor is not properly set. And Part_SimpeCopy also does not copy colors over.
I think the body needs some more logic, but not sure if it's meant to populate DiffuseColor or not? AFAIK the body has no shape either...
Or the importer simply needs to get all shapes inside a body? Not sure how to proceed here.

@donovaly
Copy link
Member

@wwmayer , what do you think?

@wwmayer
Copy link
Contributor

wwmayer commented May 18, 2022

It doesn't look like the STEP export function is broken but the Body object hasn't correctly set the colors. If you open the .FCStd file and check App.ActiveDocument.Body.ViewObject.DiffuseColor it only prints a single color value (0.8, 0.8, 0.8) which is the standard grey. For comparison the colors of App.ActiveDocument.Fillet004.ViewObject.DiffuseColor (the tip) is set to (0.33, 0.66, 1.0).
App.ActiveDocument.Fillet002.ViewObject.DiffuseColor (visible but not the tip) prints a long list of colors.

IMO the correct way should be to copy the diffuseColor property of the tip to the body object. So, if you want to export Fillet002 set it as tip and then run App.ActiveDocument.Body.ViewObject.DiffuseColor=App.ActiveDocument.Fillet002.ViewObject.DiffuseColor
Now the exported STEP file has the correct colors.

@donovaly
Copy link
Member

IMO the correct way should be to copy the diffuseColor property of the tip to the body object.

Thanks.
Could you make a PR?

@yorikvanhavre
Copy link
Member

Let me try to do it today ;) then we ask @wwmayer if I can't 😅

@yorikvanhavre
Copy link
Member

Ok I guess I need your help @wwmayer 😅
I added the following code under ViewProviderBody's updateData():

        // update DiffuseColor
        Gui::ViewProvider* vptip = Gui::Application::Instance->getViewProvider(tip);
        if (vptip && vptip->isDerivedFrom(PartGui::ViewProviderPartExt::getClassTypeId())) {
            auto colors = static_cast<PartGui::ViewProviderPartExt*>(vptip)->DiffuseColor.getValues();
            this->DiffuseColor.setValues(colors);
        }

But no effect, the DiffuseColor is still standard grey after the tip's DiffuseColor has changed (even after touching it). I guess updateData is not the right place to do it, as some GUI-only mechanism probably takes place afterwards... How would you proceed?

@wwmayer
Copy link
Contributor

wwmayer commented May 20, 2022

But no effect, the DiffuseColor is still standard grey after the tip's DiffuseColor has changed (even after touching it).

This happens when just loading the test file. However, when you set the new tip via context-menu the correct colors are set.

I guess updateData is not the right place to do it, as some GUI-only mechanism probably takes place afterwards...

It is the right place (it should be inside the if (prop == &body->Tip)) but because the Body has saved the wrong colors they will be restored after the tip property and thus overrides the correct colors again.

How would you proceed?

It's up to the user to manually change & restore the tip to correct the colors. When saving the project again the correct colors are written to the file. Or we can touch the tip inside Body::onDocumentRestored() to do this automatically.

@yorikvanhavre
Copy link
Member

Thanks @wwmayer !

ipatch pushed a commit to ipatch/FreeCAD that referenced this issue May 29, 2022
berniev pushed a commit to berniev/FreeCAD that referenced this issue Jul 16, 2022
coreyoconnor pushed a commit to coreyoconnor/FreeCAD that referenced this issue Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This issue or PR is related to a bug Color Regarding the color handling UI/UX WB Part Design Related to the Part Design Workbench
Projects
None yet
Development

No branches or pull requests

6 participants