Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Curves->Marching Squares on Surface] gap fix. #4826

Merged
merged 5 commits into from
Dec 21, 2022

Conversation

satabol
Copy link
Collaborator

@satabol satabol commented Dec 18, 2022

Windows 11, Blender 3.3.1, Sverchok-master

In node's example of [Marching Squares on Surface] there is a gap:
http://nortikin.github.io/sverchok/docs/nodes/curve/marching_squares_on_surface.html
image

and:
image

Recreated scheme of nodes:

https://gist.github.com/a7846c7f7a3c1de01b182ae4981d79c2

image

In this fix I remove a gap:

image

image

u_size = (u_max - u_min)/samples_u
v_size = (v_max - v_min)/samples_v
u_size = (u_max - u_min)/(samples_u-1)-0.0000001
v_size = (v_max - v_min)/(samples_v-1)-0.0000001
Copy link
Collaborator

@portnov portnov Dec 19, 2022

Choose a reason for hiding this comment

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

I would suggest to make this tolerance value a node setting, instead of hardcoding. Maybe hide it in node's N panel.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@portnov This is not good idea. I found real problem and wrote new fix. The description of problem:

With some T value calculation do errors:
image

The problem is (4):

u_size = (u_max - u_min)/(samples_u-1)
u_size * (samples_u-1) > (u_max - u_min) (WTF??? - float precision)

and this go to next wrong result:
image

In this situation need decrease a bit u_size and I propose do this with next code:
image

This take good result:
image

If this is not enough then this can be do with (for range) u_size/v_size to calc a value where
u_size * (samples_u-1) <= (u_max - u_min)

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MarchingSquaresOnSurface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Found some glitches. I create for-range with new formula based on:

x = min_x + x_size * x0 + x_size/2.0
y = min_y + y_size * y0 + y_size/2.0

https://gist.github.com/df4d5b173e4934aff740746f28450cf1
MarchingSquaresOnSurface fix 002

Copy link
Collaborator Author

@satabol satabol Dec 19, 2022

Choose a reason for hiding this comment

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

There is a connection in current version between these lines of code:

u_size = (u_max - u_min)/samples_u
v_size = (v_max - v_min)/samples_v
uv_contours, new_edges, _ = make_contours(samples_u, samples_v, u_min, u_size, v_min, v_size, 0, contours, make_faces=True, connect_bounds = self.connect_bounds)

x = min_x + x_size * x0 + x_size/2.0
y = min_y + y_size * y0 + y_size/2.0

x_size = u_size and on last iteration in line 31 result of x has to be u_max but with some T values result exceed u_max. This is a main problem. If refactor of make_contours function and send u_max and v_max as addition parameters and assign its in last [for-range] cycle like:

x = min_x + x_size * x0
y = min_y + y_size * y0

if x0==samples_u-1: x=u_max

if y0==samples_u-1: y=v_max

then problem will solved forever ))) But I have suppose that make_contours is used somewhere. That is why I create compensation formulas.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you can always add new parameter to make_contours if you make it optional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@portnov Thank you! It work. Can you look at that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@portnov
Copy link
Collaborator

portnov commented Dec 20, 2022

Looks good to me.

@satabol satabol merged commit a470f9d into master Dec 21, 2022
@satabol satabol deleted the curves_marchingsquares_on_surface_fix_001 branch December 21, 2022 08:39
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.

2 participants