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

webgl: use Processing's line vertex shader #2515

Merged
merged 4 commits into from
Jan 8, 2018

Conversation

Spongman
Copy link
Contributor

@Spongman Spongman commented Jan 7, 2018

ok, this one uses (a tweaked version of) Processing's line vertex shader.

(partially) fixes #2488, fixes #2510.

it doesn't display the same wireframe scaling behavior that Processing does, but it's the same as p5.js currently is.

@Spongman Spongman changed the title webgl: use Processing's line vert shader webgl: use Processing's line vertex shader Jan 7, 2018
@stalgiag
Copy link
Contributor

stalgiag commented Jan 8, 2018

also fixes #2324, which is a major bonus

So I still feel like that snapping that I mentioned in #2488 is too pronounced. You are right that Processing has it, but it is significantly less severe in Processing. Over here it feels like a too big of a problem to ignore. You can see this by rotating any primitive with a large stroke. For example this shows the effect pretty strongly:

  background(255);
  rotateX(frameCount * 0.02);
  rotateY(frameCount * 0.02);
  strokeWeight(50);
  box(300);

This shader does look better otherwise but this is definitely something we should fix before merging. We could probably move the edge toward the camera but then we will probably hit similar problems to #2510 again. I see that the problem is one half of the edge appearing suddenly but I don't understand why the billboarding is sometimes gradual as expected and sometimes it goes from full invisibility, perpendicular to camera, to being fully parallel to the camera in what seems like a single frame.

@stalgiag
Copy link
Contributor

stalgiag commented Jan 8, 2018

Looking at this some more it might be something worth pinging Processing folks about too because it seems shader specific and it definitely is noticeable on their end too now that I am remembering to account for the difference in strokeWeight

@Spongman Spongman closed this Jan 8, 2018
@Spongman Spongman reopened this Jan 8, 2018
@Spongman
Copy link
Contributor Author

Spongman commented Jan 8, 2018

yeah, the snapping is definitely bad. but it's there in master, also..

@stalgiag
Copy link
Contributor

stalgiag commented Jan 8, 2018

😅 You are very right! I was so caught up in this branch that I forgot to go back to try out the huge strokes on master. I will do a final run through and merge this tomorrow. As for the snapping, it can become a separate issue and we can reach out to the folks that work on P3D and see what they think. The problem seems just as present on Processing even if slightly mitigated by the better joins.

@Spongman
Copy link
Contributor Author

Spongman commented Jan 8, 2018

maybe one way to do this is to project the camera-facing edge quads back onto each face?

@stalgiag
Copy link
Contributor

stalgiag commented Jan 8, 2018

That's a thought. Though now that I have been playing around with Processing more it feels like less of a priority. It might make sense to figure out our joins first. This PR is super straightforward and everything seems to be working nicely with the swap out. Thanks for this!

@stalgiag stalgiag merged commit 1321e3d into processing:master Jan 8, 2018
@stalgiag stalgiag mentioned this pull request Jan 8, 2018
14 tasks
@Spongman Spongman deleted the processing-line-vert branch January 8, 2018 16:05
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.

webgl: artifact caused by edge z adjustment webgl: incorrect billboarding of thick lines
2 participants