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

Improve shading to properly display blind holes #2161

Open
hannobraun opened this issue Jan 16, 2024 · 7 comments
Open

Improve shading to properly display blind holes #2161

hannobraun opened this issue Jan 16, 2024 · 7 comments
Labels
good first issue Good for newcomers topic: display Displaying Fornjot models type: feature New features and improvements to existing features

Comments

@hannobraun
Copy link
Owner

Problem

This is how blind holes currently look:
Screenshot from 2024-01-16 11-28-41

You can reproduce this picture, by running cargo run -p holes from the repository root, then rotating (drag while left mouse button is pressed) and moving (drag while right mouse button is pressed) the model until you see the blind hole.

As you can see in this screenshot, the surface of the part and the bottom of the blind hole look exactly the same. This is because they are parallel, and the shading only depends on the angle between a surface and the viewing direction.

Solution

I'm not an expert on graphics, and I'm sure there are many ways to improve this. If you know of one, feel free to implement it!

The solution that comes to my mind, is to make shading also depend on the distance to the camera. This would have to be carefully tuned, to make cases as the one shown in the screenshot look decent, without making far-away parts completely black. It might make sense to clamp the result in such a way, that the effect of distance can never make a part too dark.

Implementation

Rendering is implemented in the fj-viewer crate, specifically within shader.wgsl. It's possible that this issue can be completely addressed with only changes to the linked shader function. It's also possible (especially, if a more advanced solution is chosen), that this requires changes to the larger graphics system.

In any case, implementing this will only require knowledge shader languages (I'm guessing that WGSL is pretty easy to understand for anyone familiar with other shader languages), and possibly Rust and wgpu. It won't require knowledge of other Fornjot internals. For this reason, I'm tagging this as a good first issue Good for newcomers .

@hannobraun hannobraun added type: feature New features and improvements to existing features topic: display Displaying Fornjot models good first issue Good for newcomers labels Jan 16, 2024
@felix91gr
Copy link
Contributor

felix91gr commented Jul 28, 2024

It's been a long while since I've done shader programming, but as far as I'm aware the canonical way that these kinds of shadows are computed and drawn is Shadow Mapping.

https://github.com/gfx-rs/wgpu/tree/trunk/examples/src/shadow I'm going to see if I can extract the design from the wgpu-rs examples, and will ping you back if I do :)

@felix91gr
Copy link
Contributor

Okay I think I'm right regarding the shadow map, but currently I'm not able to work on this.

@hannobraun
Copy link
Owner Author

Thank you for the feedback, @felix91gr, but we're talking about different things!

I'm talking about shading, not shadows. See the side walls of the hole in the screenshot above. Those are shaded (in this case made darker), based on the viewing angle of the triangles. If shading did not only depend on viewing angle, but also on distance, then the bottom of the whole would have a color that's distinct from the top surface of the part.

I don't know if shadows would be a benefit for rendering CAD models. But either way, I don't think I would want to merge them, if there was a pull request. This is just a simple renderer to augment the kernel, for testing and demonstration purposes. I'd like to leave anything that's complicated to third-party renderers.

@aabdullah27182845
Copy link

Okay... I tried to implement the change by adding the solution you mentioned earlier. I'm not good at computer graphics either, but I thought using a sigmoid adjusted upwards by 0.5 may have been a good place to start. However, I can't see the changes appearing on the model. Could someone please have a look at the changes I made to my fork? Thanks

File changes: https://github.com/aabdullah27182845/fornjot/blob/main/crates/fj-viewer/src/graphics/shader.wgsl

@hannobraun
Copy link
Owner Author

Thank you for looking into this, @aabdullah27182845! I think your code looks reasonable, and I can't tell why it's not working.

I can take a closer look next week. If you want to keep experimenting until then, my suggestion for debugging would be to set darkening_factor to a fixed value first (something extreme, like 0.0), to see if you can see any effect at all. That could show you whether it's the math that's not working, or if it's a more fundamental problem.

Also, could you submit a pull request? You can mark it as a draft, to indicate that it's not ready for merging yet. A pull request would make it easy to see and discuss changes. And, if you set the flag that enables that, even allow me to push changes to your branch, so we can collaborate easily.

@aabdullah27182845
Copy link

I've put up a draft pull request and I've tested that out too. There's something wrong with the math (either the library functions or my own implementation) since putting 0.0 makes everything black. I'm going to look more into the issue. Thanks!

@hannobraun
Copy link
Owner Author

hannobraun commented Sep 28, 2024

Nice, so at least we know that the mechanism you used works in principle. From here it's just a matter of experimenting, I guess. I'd try to start with a very simple formula, see if that works, and make it more complicated from there as required.

One thought I had since I wrote my last message here, is that we should consider what the distance value is. If it's close to 0, that would lead to a return value from exp that's close to 1, meaning the fraction would be nearly 0.5, and the whole factor would end up being close to 1. That would explain why you're seeing no difference.

Which leads to the question, how can we adapt the math, so it works with a wide range of distance values? I don't have an idea off the top of my head, but maybe we shouldn't be overthinking it. If we can come up with something that works for the current example models, that would already be an improvement. From there, we can try to come up with something more scalable, or leave that as a problem for somebody else later 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers topic: display Displaying Fornjot models type: feature New features and improvements to existing features
Projects
None yet
Development

No branches or pull requests

3 participants