Skip to content

Fix duplicate quad faces in OBJ export for triangulated quads#135

Open
alsomar wants to merge 1 commit intothomthom:1.0from
alsomar:1.0
Open

Fix duplicate quad faces in OBJ export for triangulated quads#135
alsomar wants to merge 1 commit intothomthom:1.0from
alsomar:1.0

Conversation

@alsomar
Copy link
Copy Markdown

@alsomar alsomar commented Mar 6, 2026

When exporting to OBJ, any quad face represented as two triangles (triangulated quad) was being exported as two separate quad polygons instead of one. This caused geometry duplication in the exported file.

You can find the issue here: #134. I thinks it's also the root cause of these other issues: #127, #124, #120

In EntitiesProvider#get_entity, when a triangulated quad's native faces had been previously cached as plain Sketchup::Face objects (which happens during EntitiesProvider.new), the condition guarding cache_entity prevented the created QuadFace from being stored in @faces_to_quads:

if add_to_cache && @types[ Sketchup::Face ][ entity ].nil?
  cache_entity( quad )
end

During OBJ export, write_entities calls Surface.get(native_entities, true), which groups the two triangles of a quad into a single Surface (they share a soft edge). Then entities.get(surface.faces) is called on [triangle1, triangle2].

Because neither triangle was in @faces_to_quads yet, two separate QuadFace instances were created for the same underlying triangles. Since QuadFace has no == method, .uniq could not deduplicate them, and both were written to the OBJ file.

Remove the guard condition so that cache_entity is always called when add_to_cache is true:

cache_entity( quad ) if add_to_cache

After caching the QuadFace created from triangle1, both triangle1 and triangle2 are registered in @faces_to_quads. When triangle2 is subsequently processed, it returns the same QuadFace instance, so .uniq correctly collapses the list to a single quad.

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.

1 participant