Skip to content
This repository was archived by the owner on Apr 29, 2025. It is now read-only.

fix(imports/zones): average z coordinate of non planar polys#716

Closed
JanBlk wants to merge 1 commit intooverextended:masterfrom
JanBlk:master
Closed

fix(imports/zones): average z coordinate of non planar polys#716
JanBlk wants to merge 1 commit intooverextended:masterfrom
JanBlk:master

Conversation

@JanBlk
Copy link

@JanBlk JanBlk commented Feb 27, 2025

I noticed that when creating a polyzone with points that have very similar, but not equal, z coordinates (so if they all round to the same number), the previous averaging logic threw an error, it was also overcomplicated.

@tom-osborne
Copy link
Contributor

tom-osborne commented Mar 7, 2025

This function does indeed calculate the average value of all the z-coordinates, and will prevent the error occuring when there is no duplicate z-coords.

However, this does produce a different result that the current ox implementation.

I created a simple test here.

When you provide a list of points without a duplicate z-coord, the current ox implementation breaks. But when a duplicate z-coord is present, it doesn't break, but also doesn't calculate the average. It's either the median value, or some kind of biased average.

@thelindat @DokaDoka The implementation in this PR would produce the average z-coord and prevent the code breaking, but I am unsure if average is the desired result.

Edit: On further review, it looks like current implementation is just picking the most common z-value. See #723 for alternative fix, which maintains current implementation.

Related to this Discord discussion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants