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

Coloring support #343

Merged
merged 4 commits into from
Mar 14, 2022
Merged

Coloring support #343

merged 4 commits into from
Mar 14, 2022

Conversation

therealprof
Copy link
Contributor

Implement basic coloring support for shapes

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 #291

Signed-off-by: Daniel Egger [email protected]

hannobraun
hannobraun previously approved these changes Mar 14, 2022
Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you, @therealprof!

I think this looks good, overall. There are some things that could be improved, but as you state in your last commit, let's look at those later.

Some thoughts on improvements:

  1. There should be a Color type that wraps the [u8; 4] and provides a nice API around it.
  2. I don't like how color is added to the various fj types.

Regarding 2., not sure what would be better. Maybe an explicit fj::Color operation, that colors everything it wraps (might not be flexible enough, for example regarding different colors for sweep side walls, as your comment mentions).

Or maybe there could be a type Object<T> that would wrap Shape2d/Shape3d/Shape everywhere those are used, and add color information (and in the future, potentially other information that all objects share).

But that doesn't really matter right now, I'm just throwing out ideas. I'll be merging this pull request shortly.

This adds access functions and fixes all users as well as README.md

Signed-off-by: Daniel Egger <[email protected]>
Signed-off-by: Daniel Egger <[email protected]>
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
Copy link
Owner

Hmm, GitHub wouldn't let me rebase, claiming there were conflicts, but doing so manually was no problem. Will merge as soon as CI is green.

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.

Colouring support
2 participants