Skip to content

Use simpler shader for rect-like glyphs without rounded corners - #13651

Merged
ianthomas23 merged 1 commit into
branch-3.4from
ianthomas23/rect_shader_round_or_not
Jan 23, 2024
Merged

ianthomas23 merged 1 commit into
branch-3.4from
ianthomas23/rect_shader_round_or_not

Conversation

@ianthomas23

Copy link
Copy Markdown
Member

Rect glyphs and their relatives (Block, Quad, etc) support rounded corners via the border_radius property. The WebGL backend now has full support for these rounded corners (#13649). Because of the way GLSL shaders work (massively in parallel) the rounded corner rendering code is executed even if rounded corners are not required. This is unnecessary processing for those Rect-like glyphs that don't have rounded corners.

This PR introduces a new GLMarkerType of "round_rect" for rects with rounded corners, and keeps the use of "rect" for non-rounded corners. Actually the "rect" marker type reuses the "square" shader code, so it is not even new shader code. There is a new boolean flag on BaseMarkerGL to indicate if round corners are set and this is used to determine which GLMarkerType is used for rendering.

There are no changes at all in baseline test images.

this._show.update()

this._draw_one_marker_type(marker_type, transform, main_gl_glyph)
this._draw_one_marker_type(main_gl_glyph.marker_type, transform, main_gl_glyph)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Note the marker_type is now determined after set_data is called as that is where _border_radius_nonzero is set.

@codecov

codecov Bot commented Jan 19, 2024 •

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1ce39bf) 92.55% compared to head (97eb839) 92.57%.

Additional details and impacted files
@@              Coverage Diff               @@
##           branch-3.4   #13651      +/-   ##
==============================================
+ Coverage       92.55%   92.57%   +0.02%     
==============================================
  Files             323      323              
  Lines           20525    20525              
==============================================
+ Hits            18996    19002       +6     
+ Misses           1529     1523       -6     

@mattpap mattpap left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. Is there a measurable performance difference between this and the original approach?

@ianthomas23

Copy link
Copy Markdown
Member Author

Looks good. Is there a measurable performance difference between this and the original approach?

There is, just. Using the following test code on my dev machine (Quadro T1000 graphics card, ubuntu 23.10, chromium 118) and panning in small circles using this branch gives 40-45 frames per second without rounded corners, 35-40 FPS with them. branch-3.4 gives 35-40 FPS regardless of rounded corners.

from bokeh.plotting import figure, show
import numpy as np

n = 20000
rng = np.random.default_rng(12314)
x = rng.uniform(size=n)
y = rng.uniform(size=n)

backend = "webgl"
p = figure(width=800, height=800, title=f"{backend} {n}", output_backend=backend)
p.rect(
    x=x, y=y, width=0.1, height=0.1, line_width=0, fill_alpha=0.01,
    #border_radius=20,
)

show(p)

@ianthomas23
ianthomas23 merged commit 4b61612 into branch-3.4 Jan 23, 2024
@ianthomas23
ianthomas23 deleted the ianthomas23/rect_shader_round_or_not branch January 23, 2024 09:05
Chiemezuo pushed a commit to Chiemezuo/bokeh that referenced this pull request Aug 27, 2024
@github-actions

Copy link
Copy Markdown

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 Oct 25, 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.

2 participants