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

[CSG]: Non-touching unions fail to retain some colours #227

Closed
Cloud7050 opened this issue Sep 5, 2023 · 1 comment · Fixed by #287
Closed

[CSG]: Non-touching unions fail to retain some colours #227

Cloud7050 opened this issue Sep 5, 2023 · 1 comment · Fixed by #287
Labels
Bug [Category] minor [Priority]

Comments

@Cloud7050
Copy link
Contributor

CSG currently patches the JSCAD modeling library in order to retain colour data between operations. If I recall correctly, the unmodified library resets colours to its default shade of blue after each operation.

With the patch applied, the union of two Shapes in CSG normally retains their various colours. However, the patch does not cover all cases.

If non-touching Shapes are unioned, one of them reverts to the default blue.

Snippet to reproduce issue:

import {
    silver, crimson, black,
    cube, cone, sphere,
    intersect, union, scale, translate,
    render_grid_axes
} from "csg";

const base = intersect(
    scale(cube(silver), 1, 1, 0.3),
    scale(cone(crimson), 1, 1, 3)
);
const snowglobe = union(
    translate(sphere(black), 0, 0, 0.5),
    base
);
render_grid_axes(snowglobe);

image

@Cloud7050
Copy link
Contributor Author

Another example snippet that loses colours below. There also seems to be an infinite loop bug involving union() when the Shapes are too far from each other, but I'm not entirely certain (see #231).

image

// Source §1
// Prof Martin's Sierpinski fractals

import { union, translate, scale, render, pyramid, sphere, cube }
from 'csg';

function repeat(n, trans, s) {
    return n === 0
           ? s
           : repeat(n - 1, trans, trans(s));
}

// sierpinski returns a shape transformer
// following Sierpinski's 3D fractal scheme
// v: vertical displacement of original shape
//.   for lower level shapes
// h: horizontal displacement of original shape
//.   for lower level shapes
function sierpinski(v, h) {
    return o => {
        const t1 = translate(o,  h,  h, -v);
        const t2 = translate(o, -h,  h, -v);
        const t3 = translate(o,  h, -h, -v);
        const t4 = translate(o, -h, -h, -v);
        const c = union(o,
                        union(union(t1, t2),
                              union(t3, t4)));
        const c_scaled = scale(c, 0.5, 0.5, 0.5);
        return c_scaled;
    };
}

// spheres are computationally expensive
// only try repeat 2
render(repeat(2,
              sierpinski(0.6, 0.6),
              sphere('#edd4c8')));

Cloud7050 added a commit to Cloud7050/modules that referenced this issue Sep 6, 2023
@Cloud7050 Cloud7050 added the minor [Priority] label Sep 7, 2023
martin-henz pushed a commit that referenced this issue Sep 12, 2023
* feat(src): Extract & update CSG's CSS constants

* chore(csg): Remove V1 comments referencing fork issues

* feat(csg): Migrate to BlueprintJS Tooltips

* devfeat(csg): Update samples

* devfeat(curve): Add samples

* docs(curve): Update animation function docs

* ref(csg): Remove default colour

The addition of operation colour preservation and removal of the store/render pattern means that all Shapes now require an explicit colour. Changing the default colour of the wrapped renderer no longer has a visible effect.

* fix(build): Fix `build modules` building no tabs

Revise/clarify the logic of retrieveBundlesAndTabs() and update tests/comment.

* feat(): Enhance & make curve canvases consistent

* feat(csg): Replace hint dots with colons

* ref(csg): Use real js-slang lists

* devfeat(csg): Update colours sample

* devfeat(csg): Add primitives sample

* chore(pkg): Lock modeling version for patch file

* ref(build): Reorder config to match

Left over from tests with explicit tsconfig path when troubleshooting
build errors

* ref(csg): Rename to Operable, no List, move/del funcs

* fix(csg): Fix primitive sample trailing comma

* devfeat(csg): Add Prof's Sierpinski fractal

* FEAT(csg): Overhaul functions, documentation

* devfeat(csg): Update sample import names

* ref(csg): Reorder sample functions

* devfeat(csg): Add rotation sample

* devfeat(csg): Add summary snowglobe example as sample

* docs(csg): Plural categories, bulleted list, tweaks

* ref(csg): Refactor operables

Rename params, non-deep children copy for new Group

* feat(csg): Improve error feedback for pitfalls

* fix(csg): Implement proper ungroup

* feat(csg): Improve error feedback for pitfalls

* feat(csg): Update sample utility funcs

* feat(csg): Update sierpinski sample for new primitive

* fix(rune): Fix low resolution heart

* docs(csg): Link to GitHub samples in summary

* feat(csg): Update sierpinski sample

* feat(csg): Readd custom default colour due to bug #227

* ref(build): Remove leftover from troubleshooting

* docs(csg): Tweak pyramid dimensions description

* feat(sound): Throw error on invalid sound durations

Closes #110

* chore(pkg): Remove unused lodash

* feat(curve): Add Source version to canvases sample

* feat(tabs): Remove enforced spacing of MultiItemDisplay

* ref(csg): Default colour back in constants

* fix(csg): Disallow negative scaling, update docs instead

Based on #206 (comment).

* ref(csg): Rename RGB components to values

They were called components when the params were clamped between 0-1.
Now more fitting to call them values when 0-255.
@joeng03 joeng03 mentioned this issue Mar 12, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug [Category] minor [Priority]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant