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

Add a simple subtle outline effect by drawing back faces using wireframe mode #2934

Merged
merged 5 commits into from
Dec 3, 2023

Conversation

Noisyfox
Copy link
Collaborator

@Noisyfox Noisyfox commented Nov 30, 2023

This should improve the contrast of the edges and make features more visible.

Some comparisons:
image
image

image

image
image

@Eldenroot
Copy link
Contributor

Wow, nice!!

@kylek29
Copy link

kylek29 commented Dec 1, 2023

This has been a longtime needed feature, even for PrusaSlicer. You can't see much with many colors of filament. Much appreciated.

One question, how does it look with a black filament color? Does the effect still work to make things more visible?

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 2, 2023

One question, how does it look with a black filament color? Does the effect still work to make things more visible?

That's a good question, will take a look.

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 2, 2023

image
I've updated the algorithm so that it shows white outline for dark color and black outline for light color. It's still not perfect but I think it's a lot better now.

@Noisyfox Noisyfox marked this pull request as draft December 2, 2023 04:39
@Noisyfox Noisyfox marked this pull request as ready for review December 2, 2023 08:05
@Noisyfox Noisyfox marked this pull request as draft December 2, 2023 08:41
@Noisyfox Noisyfox marked this pull request as ready for review December 2, 2023 08:52
Copy link
Owner

@SoftFever SoftFever left a comment

Choose a reason for hiding this comment

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

Awesome

@SoftFever SoftFever merged commit 0cee513 into SoftFever:main Dec 3, 2023
12 checks passed
@igiannakas
Copy link
Contributor

Think it's not working 100% right on Mac? Is it supposed to be red in colour? It also seems to change colour when I change the filament to another but the colour selection doesnt make sense :)

image

image

Also observing some polygon clipping:

image

@igiannakas
Copy link
Contributor

Also on the printer image it’s showing the lines

IMG_2680

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 3, 2023

Think it's not working 100% right on Mac? Is it supposed to be red in colour? It also seems to change colour when I change the filament to another but the colour selection doesnt make sense :)

image

image

Also observing some polygon clipping:

image

It's expected to also show these outlines in the thumbnail (because I didn't disable the outline when rendering the thumbnails), however the colors are far off. It should be black or white, not red or purple.

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 3, 2023

And yes current algorithm will cause some extra artifacts on broken model that has non-manifold edges/non-closing faces etc, though this happens on old versions as well:
image

So it's not really the issue of this PR. The only difference is now it renders not only flickering faces but some extra lines (due to the render of backface wireframes)

@igiannakas
Copy link
Contributor

And yes current algorithm will cause some extra artifacts on broken model that has non-manifold edges/non-closing faces etc, though this happens on old versions as well: image

So it's not really the issue of this PR. The only difference is now it renders not only flickering faces but some extra lines (due to the render of backface wireframes)

Gotcha. Colors though are weird. Yours look soooo much better :D

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 3, 2023

Yeah I'll take a look at the color issue.

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 3, 2023

Btw, m1 mac or intel mac?

@igiannakas
Copy link
Contributor

Btw, m1 mac or intel mac?

M1 Pro 14 inch

igiannakas added a commit to igiannakas/OrcaSlicer that referenced this pull request Dec 5, 2023
@igiannakas
Copy link
Contributor

So this one also causes significant performance issues on M1 Pro/Max laptops when rendering complex high polygon models. See comment here:

#2905 (comment)

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 5, 2023

So this one also causes significant performance issues on M1 Pro/Max laptops when rendering complex high polygon models. See comment here:

#2905 (comment)

Yeah and it also has color issue on m1 macs. I think I'll have to disable this on m1 macs then.

@igiannakas igiannakas mentioned this pull request Dec 5, 2023
2 tasks
@SoftFever
Copy link
Owner

It's not working well for some models on Windows as well.
I think we'd better make it a option and turn it off by default @Noisyfox
image

@Noisyfox
Copy link
Collaborator Author

Noisyfox commented Dec 5, 2023

It's not working well for some models on Windows as well. I think we'd better make it a option and turn it off by default @Noisyfox image

Well this is exactly what it should look like with current approach. It only adds outlines to places where front face and back face shares the same edge, not based on the relative angle between two faces.

SoftFever added a commit that referenced this pull request Dec 5, 2023
@SoftFever
Copy link
Owner

After discussing with @Noisyfox, we decided to revert this change for now.

@Eldenroot
Copy link
Contributor

Works Fine on win. What About to add a new settings to enable/disable?

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.

None yet

5 participants