Skip to content

Handle non-surface Collada geometry - #34

Open
gonzalocasas wants to merge 1 commit into
mainfrom
dae_import_fixes
Open

Handle non-surface Collada geometry#34
gonzalocasas wants to merge 1 commit into
mainfrom
dae_import_fixes

Conversation

@gonzalocasas

Copy link
Copy Markdown
Member

UR16e DAE files contained non-surface geo that broke the parsing, added handling for that and also more material support.

What type of change is this?

  • Bug fix in a backwards-compatible manner.
  • New feature in a backwards-compatible manner.
  • Breaking change: bug fix or new feature that involve incompatible API changes.
  • Other (e.g. doc update, configuration, etc)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I added a line to the CHANGELOG.md file in the Unreleased section under the most fitting heading (e.g. Added, Changed, Removed).
  • I ran all tests on my computer and it's all green (i.e. invoke test).
  • I ran lint on my computer and there are no errors (i.e. invoke lint).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary documentation (if appropriate)

@yck011522

Copy link
Copy Markdown
Contributor

I am not very familiar with these mesh imports handler, but the fixes look good to me (if it works).

Base automatically changed from tool-base-frame to main August 7, 2026 11:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the COLLADA (.dae) mesh importer to tolerate non-surface/empty geometry nodes (e.g., lines-only or empty meshes) that previously caused imports to abort, and expands material color parsing beyond Phong to additional shader models.

Changes:

  • Skip COLLADA geometries that contain no supported surface primitives (triangles/polylist) and only raise an error if the entire file yields no surface meshes.
  • Parse material colors from Lambert, Blinn, and Constant shaders in addition to Phong.
  • Add a regression test + fixture DAE covering mixed surface, line, and empty geometries with a Lambert diffuse color.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
tests/test_resources.py Adds regression test asserting non-surface geometry is ignored and Lambert diffuse color is loaded.
tests/fixtures/surfaces_lines_and_empty_geometry.dae New minimal COLLADA fixture containing triangles + lines-only + empty geometry and a Lambert material.
src/compas_robots/resources/mesh_importer.py Updates COLLADA parsing to skip unsupported geometry primitives and broaden shader color extraction.
CHANGELOG.md Documents the importer fix and expanded shader support in Unreleased/Fixed.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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.

3 participants