tataratat / splinepy

Library for prototyping spline geometries of arbitrary dimensions and degrees, and IGA
https://tataratat.github.io/splinepy
Other
47 stars 13 forks source link

Error in show_options, colors are ignored for some reason #315

Open jzwar opened 1 year ago

jzwar commented 1 year ago

If colors are set differently for the knots and control points via keyword arguments, these will be ignored if the color c is stated. Here is an example:

import splinepy as sp

# Options
options = {
    "control_points": True,
    "control_point_ids": False,
    "control_mesh_c": "k",  # Will be ignored
    "control_point_r": 20,
    "control_point_c": "k",  # Will be ignored
    "c": (0, 240, 156),  # Uses this instead : why?
    "lighting": "off",
}

deformation_function = sp.BSpline(
    degrees=[2, 1],
    knot_vectors=[[0, 0, 0, 0.5, 1, 1, 1], [0, 0, 1, 1]],
    control_points=[
        [0.0, 0.0],
        [0.5, 0.2],
        [1.0, 0.],
        [1.5, 0.2],
        [0.0, 1.0],
        [0.5, 1.2],
        [1.0, 1.0],
        [1.5, 1.2],

    ]
)
deformation_function.show(**options)

Results in:

image
jzwar commented 1 year ago

It seems that the function eval in make_showable keeps track of the results. I think that the key words are just passt without checks to the vedo interface. Maybe we should not use vedo-keywords as show_options. Use spline_c / spline_color / 'base_color` instead? The keywords are not really consistent. We use full snake-case for some keys but abbreviations for others.

j042 commented 1 year ago

If you use show_options to set options, it will show you as expected. To answer # Uses this instead : why? - as **kwargs are passed to gus.show, it overwrites all the options that's applicable to sampled objects. c is a very common option. We can stop processing show_options from kwargs to avoid confusion.

jzwar commented 1 year ago

I know that c is applicable to points and meshes as well, which is exactly why I think we should rename the spline color options c, alpha, etc. to unique splinepy option names. Having duplicates is confusing imo, or we have to set vedo options more explicitly, with something like 'vedo_options=dict`

j042 commented 1 year ago

That's a problem related to using the workflow gus.show(..., **show_options). You can maybe edit that part

j042 commented 8 months ago

This may be - "this is a feature not a bug" However, I see that we may want some lazy-but-accurate show_options options. Some thoughts: