fix boolean operations: use topological merge instead of spatial merge

[Zadfar]

This PR aims to fix the broken boolean operations, mainly difference(exclude) and subtract.

The other issues with subtract mentioned by @0HyperCube have also been fixed.
The topological merge by distance function is more suitable to remove the duplicated points from the result of boolean operations.
Fixes #4513

[cubic-dev-ai]

No issues found across 1 file

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

_{Re-trigger cubic}

[0HyperCube]

I would consider this a hacky workaround rather than a proper fix.

You can still get the buggy behaviour by inserting a « Merge by Distance » node after the « Boolean Operation ».

A proper fix would address the faulty rendering algorithm. Specifically Vector::construct_face is incredibly baffling. It takes the next FaceSide (aka segment) from a list of adjacent edges ordered by the angle of the tangent:

Graphite/node-graph/libraries/vector-types/src/vector/vector_attributes.rs

Lines 1144 to 1154 in eb4a122

	let neighbors = &adjacency[next_vertex];
	let side_index = neighbors.iter().position(|s| {
	FaceSide {
	segment_index: s.segment_index,
	reversed: !s.reversed,
	} == side
	})?;
	side = neighbors[(side_index + 1) % neighbors.len()];
	if side == first {
	return Some(());
	}

This leads it to sometimes never produce a face because it goes into a cycle e.g.

It will explore the following segments:

Visiting FaceSide { segment_index: 0, reversed: false }
Visiting FaceSide { segment_index: 1, reversed: false }
Visiting FaceSide { segment_index: 2, reversed: false }
Visiting FaceSide { segment_index: 3, reversed: false }
Visiting FaceSide { segment_index: 5, reversed: false }
Visiting FaceSide { segment_index: 6, reversed: false }
Visiting FaceSide { segment_index: 7, reversed: false }
Visiting FaceSide { segment_index: 8, reversed: false }
Visiting FaceSide { segment_index: 3, reversed: true }
Visiting FaceSide { segment_index: 2, reversed: true }
Visiting FaceSide { segment_index: 1, reversed: true }
Visiting FaceSide { segment_index: 0, reversed: true }
Visiting FaceSide { segment_index: 4, reversed: true }
Visiting FaceSide { segment_index: 8, reversed: true }
Visiting FaceSide { segment_index: 7, reversed: true }
Visiting FaceSide { segment_index: 6, reversed: true }
Visiting FaceSide { segment_index: 5, reversed: true }

Before giving up because of the iteration limit.

This yields no faces although there clearly should be.

Please mark as draft.

[Zadfar]

Thanks for the feedback. I'll spent some time getting to know the vector rendering code better.

A proper fix would address the faulty rendering algorithm. Specifically Vector::construct_face is incredibly baffling. It takes the next FaceSide (aka segment) from a list of adjacent edges ordered by the angle of the tangent:

This confused me too. I'll experiment a bit and find where it breaks.

[0HyperCube]

Hi, thanks for the response.

I've done some sketching and I'm even more convinced that the Vector::construct_face is nonsensical.

It doesn't really make sense to look at the tangents when they can collide or overlap. For example this rectangle with a bézier through (with one handle snapped to the edge) occasionally has a broken fill rectangle_with_bézier.graphite:

Some things I've observed:

• Probably ignore bézier handles
• Probably want faces with as few anchors/segments as possible
• One face should contain a segment at most once (currently can contain the segment in both directions but this is bad).
• A segment is part of at most 2 faces (however it is possible to get stuck if you do some bad faces and follow this rule).
• A relatively simple graph has very many combinations for faces

I'm happy to discuss or collaborate on this here or on discord.

[Zadfar]

It doesn't really make sense to look at the tangents when they can collide or overlap. For example this rectangle with a bézier through (with one handle snapped to the edge) occasionally has a broken fill

I agree, the ordering should be based on something else.
While reading up on how inkscape does its vector rendering i came across this. Boolean operations and such are performed on the planar eulerian graph generated using this, instead of directly on the original graph. Would a similar approach make sense here?

I havent gotten around to how exactly it should be ordered. The bezier handles should probably be ignored as you said.
I am fairly new to this. so forgive me if this deviates from the intended design of graphite.

[0HyperCube]

While reading up on how inkscape does its vector rendering i came across this

In graphite, the boolean operations are carried out with the linesweeper crate which does a similar thing by converting intersections to vertices and then stitching the path back together.

the planar eulerian graph

Currently it seems that the Vector type models arbitrary graphs (since users can create or delete segments/points using the « Path » node via the path tool. The graphs passed to the renderer therefore do not necessarily have vertices at intersections or any other nice properties. I'm not sure if it is desirable to keep such functionality since it seems to make processing rather difficult. It is desirable to keep rendering simple and fast.

forgive me if this deviates from the intended design of graphite.

I am not a maintainer and have no clue about the intended design. I think there was some idea that the user could choose to fill or not fill individual regions like in figma. However I'm not really sure if that is still being considered. You'd probably have to consult with Keavon on this matter.