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

Colouring support #291

Closed
therealprof opened this issue Mar 6, 2022 · 9 comments · Fixed by #343
Closed

Colouring support #291

therealprof opened this issue Mar 6, 2022 · 9 comments · Fixed by #343
Labels
topic: core Issues relating to core geometry, operations, algorithms topic: display Displaying Fornjot models type: feature New features and improvements to existing features

Comments

@therealprof
Copy link
Contributor

It would be great if Fornjot would have support for specifying custom colors per object in the rendering rather than the default of red with full opacity. This might also be useful to figure out which specific item is causing rendering issues.

There's a number of issues to sort out though:

  • Which type to use? I'm not super keen on using the current [f32;4] everywhere since it doubles up the issues FJ already has with the floating point representation of coordinates and would propose [u8; 4] instead.
  • Currently vertices are compared for equality, adding a colour to a vertex would not only mean more iffiness with the passthrough of the color information but also create ambiguities with respect to "is an otherwise identical Vertex with a different colour a different Vertex or the same"?
  • I suppose a Colour type would live in it's own module to allow convenient use and manipulation. What's the best location for it?
@hannobraun
Copy link
Owner

Thank you for opening this issue, @therealprof! Support for colors is definitely a welcome feature.

I've left thoughts in my comments below.

Which type to use? I'm not super keen on using the current [f32;4] everywhere since it doubles up the issues FJ already has with the floating point representation of coordinates and would propose [u8; 4] instead.

I don't really have an opinion here. I don't foresee any problems of using floating point numbers for colors, but [u8; 4] is fine too.

Currently vertices are compared for equality, adding a colour to a vertex would not only mean more iffiness with the passthrough of the color information but also create ambiguities with respect to "is an otherwise identical Vertex with a different colour a different Vertex or the same"?

I don't think vertices are the right place to track information about color (or more generally, material). It's where the color ends up on the graphics side, but I don't think it makes sense to store color on vertices in the kernel, nor does it make sense on the model side to specify color per-vertex.

Eventually, we might want to have a concept of solids, and track material information there. But for now, I think kernel::topolopgy::faces::Face would be a fitting place.

On the model side, users could specify an optional color for fj::Sketch, which would then get inherited through extruding. Once we have support for querying models, this would open the way for overriding color on a per-face basis.

  • I suppose a Colour type would live in it's own module to allow convenient use and manipulation. What's the best location for it?

I suppose that the same type would be used in the fj crate and in the kernel. I think that makes the fj crate the most practical choice for a location.

@hannobraun hannobraun added topic: display Displaying Fornjot models topic: core Issues relating to core geometry, operations, algorithms topic: model type: feature New features and improvements to existing features labels Mar 7, 2022
@therealprof
Copy link
Contributor Author

Hi, thanks for the comments, I'll check the remainder when I find some time; this one stuck out though:

I don't really have an opinion here. I don't foresee any problems of using floating point numbers for colors, but [u8; 4] is fine too.

The problems with floats is the same as floats anywhere else: Hashing, Ordering, Equality becomes a major pain and in case of Colour I don't see the benefit of this self-inflicted pain when we can simply convert the Colour to float before handing it off to the rendering. Of course that doesn't mean that we can't extend this later on to support fancier mechanisms but I wouldn't like to make things hard from the beginning without any real benefit.

@hannobraun
Copy link
Owner

Well, hashing/ordering/equality is taken care of by Scalar. Of course there are higher-level problems that the Scalar type can't fix, and for geometry, I think measures like vertex validation (#242) are going to provide a solution. But colors? I don't see any need to check and compare those, at least not initially. So I don't see the disadvantages here.

But those arguments just support my position of having no clear preference. I'm still totally fine with [u8; 4], and don't see any reasons why we should prefer floating point numbers either.

@therealprof
Copy link
Contributor Author

therealprof commented Mar 7, 2022

I don't see any need to check and compare those, at least not initially. So I don't see the disadvantages here.

If you want to have Shapes with Colours/Material they probably will have to be hashable and orderable since those are likely to end up in data structures which enforce that requirement.

@hannobraun
Copy link
Owner

As I said, Scalar is hashable/orderable, and thus usable in such data structures. But so is u8.

@therealprof
Copy link
Contributor Author

Yes, but Scalar seems to be a bit shaky still and I wouldn't really want to rely on that for colour really. Actually I'd rather prefer FJ internally used fixed point arithmetic rather than f64/f32 internally but that's a different discussion.

@hannobraun
Copy link
Owner

Actually I'd rather prefer FJ internally used fixed point arithmetic rather than f64/f32 internally but that's a different discussion.

I'm open to that, but I haven't thought a lot about that, and am not clear on what the consequences would be. But yeah, that is a different discussion 😄

@therealprof
Copy link
Contributor Author

On the model side, users could specify an optional color for fj::Sketch, which would then get inherited through extruding. Once we have support for querying models, this would open the way for overriding color on a per-face basis.

So I was looking into this. The rendering side of things looks somewhat straight forward but I've not a clue on how to add support on the modelling side and carry that over to the kernel.

@therealprof
Copy link
Contributor Author

#338 moves the color definition of the vertices one level closer to the actual model. Once we have the ability to specify plain colors we could bump it up a notch and turn this a proper material definition and add more features.

I noticed that the models render much nicer with a slight amount of transparency added but I'm on the fence on whether it is worth it to make this as a hack to the renderer to allow for toggling it on or off or whether we simply "wait" until the models gain the ability to specify the color (and transparency).

therealprof added a commit to therealprof/Fornjot that referenced this issue Mar 13, 2022
The examples have been extended to demonstrate the coloring of objects.
A few bits and pieces are probably still missing or could be improved
but let's look at those in another step.

Closes hannobraun#291

Signed-off-by: Daniel Egger <[email protected]>
therealprof added a commit to therealprof/Fornjot that referenced this issue Mar 13, 2022
The examples have been extended to demonstrate the coloring of objects.
A few bits and pieces are probably still missing or could be improved
but let's look at those in another step.

Closes hannobraun#291

Signed-off-by: Daniel Egger <[email protected]>
hannobraun pushed a commit to therealprof/Fornjot that referenced this issue Mar 14, 2022
The examples have been extended to demonstrate the coloring of objects.
A few bits and pieces are probably still missing or could be improved
but let's look at those in another step.

Closes hannobraun#291

Signed-off-by: Daniel Egger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Issues relating to core geometry, operations, algorithms topic: display Displaying Fornjot models type: feature New features and improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants