cadquery icon indicating copy to clipboard operation
cadquery copied to clipboard

Support for materials

Open adrianschneider94 opened this issue 7 months ago • 23 comments

Implemented material support. In this approach, materials are stored in python dataclasses, the OpenCascade objects are only created when building the assembly. Tests/docs are missing at the moment.

adrianschneider94 avatar Apr 15 '25 20:04 adrianschneider94

This looks great, I wonder if there is ever a use case for a material to be a string in as an option in addition to these nice material classes you have here. I was wondering if something like this would be possible / useful

assy = cq.Assembly()
assy.add(cube, name="red_cube", material="steel")

shimwell avatar Apr 16 '25 07:04 shimwell

@shimwell it would certainly be nice to have some predefined materials, and maybe allow to use the material name for other things too. But let's start with making CI green.

adam-urbanczyk avatar Apr 16 '25 08:04 adam-urbanczyk

Codecov Report

Attention: Patch coverage is 98.50187% with 4 lines in your changes missing coverage. Please review.

Project coverage is 95.74%. Comparing base (f754469) to head (4a0bc97). Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
cadquery/materials.py 98.80% 0 Missing and 2 partials :warning:
cadquery/occ_impl/assembly.py 96.61% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1815      +/-   ##
==========================================
+ Coverage   95.66%   95.74%   +0.07%     
==========================================
  Files          28       29       +1     
  Lines        7431     7606     +175     
  Branches     1122     1150      +28     
==========================================
+ Hits         7109     7282     +173     
  Misses        193      193              
- Partials      129      131       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Apr 17 '25 20:04 codecov[bot]

Material support in step exporting will get better in OCCT: https://github.com/Open-Cascade-SAS/OCCT/pull/447

adrianschneider94 avatar Apr 18 '25 09:04 adrianschneider94

This looks great, I wonder if there is ever a use case for a material to be a string in as an option in addition to these nice material classes you have here. I was wondering if something like this would be possible / useful


assy = cq.Assembly()

assy.add(cube, name="red_cube", material="steel")

Would probably nice to have a set of materials to get started, but I think, any real-world application will require custom material sets. Maybe there's another solution and we can find an existing material repository that can be utilized.

(edit: for example https://physicallybased.info)

adrianschneider94 avatar Apr 18 '25 20:04 adrianschneider94

VRML export runs, but is not correct yet.

adrianschneider94 avatar Apr 19 '25 08:04 adrianschneider94

The PR would be ready from my side for a first iteration.

I think it would be great to add textures later on, one could create quite good visualizations directly with cadquery/vtk. For the time, the PBR materials already achieve quite nice results.

adrianschneider94 avatar Apr 28 '25 14:04 adrianschneider94

Thanks for all the hard work! Are the new classes picklable?

adam-urbanczyk avatar Apr 28 '25 15:04 adam-urbanczyk

Yes, they are pure dataclasses/basic python types, so everything can be pickled.

How far is the progress on the serialization topic? Have you thought about using pydantic? I plan to implement model caching in our internal project and json would be easier to use in a db like with Postgres' JSONB column or redis' JSON datatype compared to pickle.

adrianschneider94 avatar Apr 28 '25 17:04 adrianschneider94

Perfect, thanks!

There are no plans to implement serialization beyond pickling which is already there. For caching I'd suggest joblib, but maybe you have something else in mind.

adam-urbanczyk avatar Apr 28 '25 17:04 adam-urbanczyk

@adrianschneider94 @adam-urbanczyk What action items are left to do before getting this PR merged?

jmwright avatar May 22 '25 19:05 jmwright

Review... Given the size of the PR it might take a while, but we'll definitely get this merged.

adam-urbanczyk avatar May 22 '25 19:05 adam-urbanczyk

I'll plan to spend some time reviewing it next week then.

jmwright avatar May 22 '25 20:05 jmwright

I see that this branch has conflicts now that I must have missed last week. Should it be rebased?

jmwright avatar May 30 '25 12:05 jmwright

I see that this branch has conflicts now that I must have missed last week. Should it be rebased?

Rebase is done.

adrianschneider94 avatar Jun 02 '25 11:06 adrianschneider94

Hey, thanks for all the hard work, but this requires some serious rework with backward compatibility in mind. May I as why there are so many changes that are not needed?

I think there are only two changes that affect backwards compatibility: The missing toTuple() method and the changed signatures of some export methods. Or am I missing something?

The only changes not needed (in my eyes) are the assembly iteration and unifying the color handling. I think both make sense in terms of readability/clarity and as they appear in all places where materials need to be handled, it seemed obvious to me to handle it.

As a suggestion, I will revert the iteration and widen the signatures of all places where colors can be passed, so it's entirely backwards compatible at all those places.

adrianschneider94 avatar Jun 02 '25 12:06 adrianschneider94

@adrianschneider94 I see the CI is green, congrats on that.

I'm super keen on this feature.

The part I'm most keen on is allowing a simple string for a material. We noticed it is possible to do this using a Material

assy = cq.Assembly()
assy.add(cube, name="red_cube", material=Material("steel"))

I was wondering what are you thoughts on allowing a string as well as Material object, something like this?

assy = cq.Assembly()
assy.add(cube, name="red_cube", material="steel")

We noticed reviewer edits on the branch are disabled so I guess you are the only one who could make these changes if you consider it acceptable.

Anyway it is a nice PR and I would love to see the features suggested in Cq

shimwell avatar Aug 08 '25 07:08 shimwell

@shimwell, I'll try to get back to it in the next weeks. Regarding the string materials: What materials would you include, should they be maintained in cadquery? Or would you include a material repository as dependency or fetch from the web? (see also https://github.com/CadQuery/cadquery/pull/1815#issuecomment-2816127608)

adrianschneider94 avatar Aug 13 '25 12:08 adrianschneider94

Thanks @adrianschneider94

Regarding the string materials: What materials would you include, should they be maintained in cadquery? Or would you include a material repository as dependency or fetch from the web? (see also #1815 (comment))

I would have a local definition of materials. I've not got a fixed list of what string correlates to what color as I don't intend to use then for colors. For me the material strings would be used to look up elemental composition and density of the material for use in physics simulations.

shimwell avatar Aug 13 '25 12:08 shimwell

Two questions @adrianschneider94. Could you enable edits by maintainers? Do you intend to address the review comments?

adam-urbanczyk avatar Aug 14 '25 06:08 adam-urbanczyk

I think it would be better from a compatibility point of view, to have material as a key in the metadata dict for each component of the assembly. This way the call signatures to iterators are not changed by the addition of materials.

lenianiva avatar Aug 31 '25 05:08 lenianiva

Metadata are meant for custom metadata (which will be eventually supported when exporting to STEP or CAF). Anyways, we will handle this PR after the release. Most likely it will be significantly reworked and split up.

adam-urbanczyk avatar Sep 09 '25 06:09 adam-urbanczyk

The option to allow edits does not appear, I think because the repo is owned by an organization: https://github.com/orgs/community/discussions/5634

Yes, I will address the comments, when I have time again to work on this.

adrianschneider94 avatar Sep 17 '25 07:09 adrianschneider94