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 rendering of Ngon glyph #14044

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Conversation

ianthomas23
Copy link
Member

This implements WebGL rendering of the new Ngon glyph.

Here is the new visual test image, you can tell by the hatching pattern that WebGL is used on the right:

Glyph_models__should_support_Ngon

In terms of implementation I have moved most of the existing CircleGL into its new base class RadialGL which is also the base class for the new NgonGL class. For the GLSL shaders I have reused the existing marker shaders adding a new marker type "ngon". This allows us to reuse the hatching, line joins, etc. The only thing not supported is line dashes, which is not supported for any of the markers.

For the new visual test I've had to add a new Figure.ngon function to glyph_api.ts. I haven't done this before so I've copied, pasted and modified the circle code for this. Someone with more experience of this should probably check that I have done this correctly.


if (a_show < 0.5 || v_size.x < 0.0 || v_size.y < 0.0 || (v_size.x == 0.0 && v_size.y == 0.0)
#ifdef USE_NGON
|| v_n < 3.0
Copy link
Member Author

Choose a reason for hiding this comment

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

This line avoids attempting to render an Ngon with n<3. I have not added an explicit visual test for this.

@mattpap mattpap added this to the 3.6 milestone Aug 27, 2024
@mattpap
Copy link
Contributor

mattpap commented Aug 27, 2024

For the new visual test I've had to add a new Figure.ngon function to glyph_api.ts

This should have been added in the original PR. In anycase, looks fine. In general, as long as the signature matches bokeh's Python API, then it should be fine.

@bryevdv
Copy link
Member

bryevdv commented Aug 27, 2024

This should have been added in the original PR.

Yup I just forgot about it, thanks for adding @ianthomas23 !

@ianthomas23 ianthomas23 merged commit a28073d into branch-3.6 Aug 28, 2024
22 checks passed
@ianthomas23 ianthomas23 deleted the ianthomas23/14035_webgl_ngon branch August 28, 2024 16:19
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add WebGL implementation for Ngon glyph
3 participants