Redesign WebGL backend using ReGL

[ianthomas23]

This is WIP to replace the previous webgl line code with instanced line rendering using the regl javascript library. I have rewritten both vertex and fragment shaders, and regl now does most of the webgl heavy lifting of creating vertex buffers, textures, etc. The new code is simpler, hopefully easier to understand, and there is a significant reduction in the amount of data sent from CPU to GPU which will eventually give a performance benefit.

Above example for solid lines was produced by the following code:

from bokeh.core.enums import LineCap, LineJoin
from bokeh.io import show
from bokeh.layouts import row
from bokeh.plotting import figure, curdoc
import numpy as np

w = 500
h = 600
x = np.asarray([0,   1,   2, 3, 4])
y = np.asarray([1.2, 1.5, 1, 4, 1.5])

fig0 = figure(plot_width=w, plot_height=h, output_backend='canvas', title='canvas')
                            
fig1 = figure(plot_width=w, plot_height=h, output_backend='webgl', title='webgl',
              x_range=fig0.x_range, y_range=fig0.y_range)

linewidths = [20, 3]
caps = [LineCap.butt, LineCap.round, LineCap.square]
joins = [LineJoin.miter, LineJoin.round, LineJoin.bevel]
colors = ['red', 'green', 'blue']

for fig in (fig0, fig1):
    offset = 0.0
    for lw in linewidths:
        for cap, join, color in zip(caps, joins, colors):
            offset -= 1.0
            fig.line(x, y + offset, line_color=color, line_width=lw,
                     line_alpha=0.6, line_cap=cap, line_join=join,
                     legend_label=f'cap={cap}, join={join}, lw={lw}')
    fig.legend.location = 'top_left'

curdoc().add_root(row(fig0, fig1))

The output looks identical to my naked eye; there are some slight differences in antialiasing at the edge if you zoom in.

Dashed lines still need some more work: neither the start offset or user-defined offsets are correct, and joins are not correct either. But it almost acceptable for thin lines.

I've tried to keep the code architecture similar to before as it is a big enough change without also changing the architecture, and I wanted to leave the (unmodified) marker webgl code still working. But I haven't succeeded here, if you use both the old webgl markers and new webgl lines on the same canvas strange things happen presumably through sharing webgl resources that are initialised in two incompatible ways. So the webgl marker code needs to be changed in tandem with this, either in this PR or another one soon after.

My webgl initialisation is also poor, if you don't have the correct extensions it fails ungracefully. Fixing this will be easy when the markers are changed.

Things I know I need to do:

1. Fix the dashes.
2. Add tests (!)
3. Webgl markers and initialisation.
4. Look into pixel_ratio which I have ignored so far.

I am interested in what others think might be problems that I haven't thought of, e.g. multiple renderings of the same data, interactive changes. Also my typescript isn't very good yet, I am learning it quickly but I am at the stage of it being both really useful and really annoying compared to pure javascript, so I am expecting to have to make changes here.

If anyone wants to compile and play with the code, there are a couple of variables in webgl/line_gl.ts that are interesting to change: setting debug_show_mesh on line 52 to true will render the webgl mesh on top of the lines, and antialiasing on line 49 can be altered to vary the width of the antialiasing.

[ianthomas23]

Am I on the right track here? It would be nice to receive some feedback before I continue with it.

[mattpap]

@ianthomas23, we are currently finalizing a release, so this will have to wait until we're done. In any case, this is definitively a good start. In the meantime I would add extensive visuals tests (different sizes, joins, patterns, overlap, etc.) (including comparing to canvas; though this doesn't imply it has to match perfectly) and fix the any type situation (regl has pretty good type declarations; based on a cursory review).

[mattpap]

(...) But it almost acceptable for thin lines.

I think this what we care most about in this case. Thick lines are pretty rare. Also if we decide that webgl is not a drop-in replacement for canvas, then there may not be an issue altogether.

[bryevdv]

@ianthomas23 sorry for the delay, as @mattpap mentioned we mostly need to get though a 2.3 release and then we can put more attention in to other waiting work.

One little suggestion I do have off the bat is to rename line.frag and line.vert. In a case like this where the implementation is being wholesale replaced, the GH diff is actually very unhelpful. I'd rather just be able to look at and follow the new implementation on its own.

[ianthomas23]

@mattpap and @bryevdv Thanks for the suggestions. I will crack on with it this week.

[mattpap]

@ianthomas23, you need to rebase this on top of branch-2.3 to allow CI tests to run.

[ianthomas23]

Rebased.

I was planning to fix the dashed lines before I add new tests and check the existing ones.

[ianthomas23]

Dash offsets and joins with butt end caps are done:

It is getting hard to tell the difference between canvas and webgl here. I need to tweak the antialiasing at the joins a little, and joins with round or square end caps are not good enough yet.

I intend to write the tests and look at markers before improving dashes further. I need a break from dashes 😁

[bryevdv]

@ianthomas23 this is looking great! We are finally at a place to cut a release candidate in the next day or so and release early next week so I very much look forward to being able to move on this more directly on branch-2.4 after that!

[bryevdv]

@ianthomas23 what are your preferences here: would you like to see instanced lines work merged earlier, or do you want to try to have a single PR that handles lines and markers w/ ReGL at once? Now that 2.3 is out I definitely want to help you move this forward as fast as you can accommodate.

Edit: I spoke too soon, looks like marker work was already added? Let us know what help/support you can best use at this point. Very excited about this!

[ianthomas23]

@bryevdv Yes, the markers use regl now too. I hadn't intended to do this yet, but it turned out to be easier than trying to get lines and markers working with different webgl initialisation. Functionally, the PR is > 90% there. The biggest thing left to do is update the existing tests and add comprehensive new ones. I've been working on these this evening, but then also decided to rebase to branch-2.4 and am stuck in a hole of git conflicts that I am dealing with slowly and carefully!

There's a bunch of smaller things I need to do, but I'm confident they aren't too tricky. I will need some help on some TypeScript issues that I'm still not happy with, and I'm failing a test that I don't fully understand. Maybe we can talk about those next week after I've dealt with the other stuff.

After this PR I'll need to do another one to tweak the existing markers and add all of the missing ones, and there is probably another PR to improve dashed lines with square/round end caps. After that I can move onto filled polygons, etc. Probably some of the architecture of what I've done isn't ideal, but I think the best plan is to stick with what we have until I've added more webgl and am more familiar with the rest of bokeh. Just after a release seems a good time to start making such changes.

I've been watching from afar the work that you and @mattpap have been doing to get 2.3 out of the door, and I clearly underestimated the amount of work that would be. You both deserve a rest!

Finally, off-topic here but I expect of interest is that I have started to work on packaging the C++ contour calculation code as a separate module. Early days yet, but you can add it to the bokeh roadmap for this year.

[mattpap]

This seems an unnecessary dependency. If you're using this in your own workflow, you should npm install -g ci.

[ianthomas23]

Yes, it's a mistake. I need to go through the package changes carefully as there are some I added that I later decided I didn't need but I may not have removed them fully.

[mattpap]

(val) => val/255

[ianthomas23]

OK.

[mattpap]

It would be better to not provide an explicit type in those two cases. You will get the precise types from type inference, which will not allow you to accidentally provide invalid cases later on.

[ianthomas23]

OK.

[ianthomas23]

I could do with some help. I've had to rework the marker webgl typescript to deal with zooming and selection using indices. This is fine for non-circle markers, but circles are causing me problems. I assumed it was due to their size and radius duality, but on closed inspection I'm seeing things I don't understand. If I add a single circle to a figure (canvas or webgl) then I see 4 separate calls to CircleView.initialize, only the first of which has a non-null _data_init. Below is the function where the equivalent webgl circle is created, using a regl-updated equivalent of this code from branch-2.4:

bokeh/bokehjs/src/lib/models/glyphs/circle.ts

Lines 37 to 44 in 346e694

	initialize(): void {
	super.initialize()
	const {webgl} = this.renderer.plot_view.canvas_view
	if (webgl != null) {
	this.glglyph = new MarkerGL(webgl.gl, this, "circle")
	}
	}

Hence there are some not-fully initialized webgl circles which cause problems. I don't understand the 4 calls to CircleView.initialize, non-circle markers do not do this.

I also note that I've messed up my rebase from branch 2.3 to 2.4. Now fixed.

[mattpap]

Now fixed.

It appears that not quite yet, as "Bring 2.4 branch up to date" commit is still there. Reset your local branch-2.4, with

git fetch
git checkout branch-2.4
git reset --hard origin/branch-2.4 

and then

git rebase --onto branch-2.4 d065e9b~1

[mattpap]

@ianthomas23, besides the size issue, what else needs to be done in this PR, or is it ready at this point?

[ianthomas23]

I've added visual tests comparing canvas, svg and webgl side-by-side, e.g.

Solid lines are very good, dashed lines vary (thin lines are good, thick lines with e.g. round caps are not good), and markers are no better or worse than they used to be. I have a list of 15 improvements I'd like to make from the minor (antialiasing of thin solid lines that are almost horizontal or vertical needs tweaking) to the major (some marker types missing), and a few unrelated but notable ones (svg backend does not use line_dash_offset).

My preference is to try to bring this to a close and deal with those 15 issues (and whatever new ones arise) separately later. In other words, consider this a "transition to regl" PR rather than a "fix webgl for lines and markers" PR.

There are 2 other issues that do need dealing with. I've partially written a canvas vs webgl comparison test with the idea of storing canvas baseline images and using CI to compare newly-generated webgl images against those canvas baselines. But this doesn't fit with the existing image comparisons

bokeh/bokehjs/test/devtools/devtools.ts

Lines 435 to 439 in 50b88be

	if (pixels > threshold) {
	await write_image()
	status.failure = true
	status.image_diff = diff
	status.errors.push(`images differ by ${pixels}px (${percent.toFixed(2)}%)${i != null ? ` (i=${i})` : ""}`)

which are, I think, based on number of different pixels exceeding a threshold. A better option here for canvas-webgl comparisons is to expect near-identical RGB but allow differing alpha as the differences are primarily due to antialiasing. This looks like a non-trivial addition to the testing code.

Secondly, as discussed previously regl has made the minimized javascript too big. I haven't yet looked at whether it is possible to reduce this given the possibilities of a separate webgl bundle. But I don't really know what that involves.

[bryevdv]

My preference is to try to bring this to a close and deal with those 15 issues (and whatever new ones arise) separately later. In other words, consider this a "transition to regl" PR rather than a "fix webgl for lines and markers" PR.

👍 from me

[mattpap]

@ianthomas23, so let's update bundle sizes to fix js-test, and we will proceed with this PR.

[ianthomas23]

so let's update bundle sizes to fix js-test, and we will proceed with this PR.

@mattpap OK, I will do that. I'll also remove the canvas vs webgl comparison that I started and put it on my "todo" list instead.

Should I squash the commits?

[bryevdv]

Should I squash the commits?

@ianthomas23 We normally "Squash and merge" PRs so it does not matter so much on our end. If you want a tidier commit list in the merge commit message, would be the only reason.

[mattpap]

For the sake of compatibility with 2d canvas, this should be pattern = concat(pattern, pattern). Though it may not happen at all, because odd patterns should be normalized at an earlier stage, thus a simple assertion would also suffice.

[mattpap]

Until/unless we can jettison large chunks of canvas-related code in favor of always using webgl, (...)

As long as we support SVG output, which seems to be a pretty popular feature, we can't remove any canvas related code, because it's responsible for SVG output as well. Besides this, canvas rendering related code is small compared to other code paths, like hit testing.

[mattpap]

(...) which are, I think, based on number of different pixels exceeding a threshold. A better option here for canvas-webgl comparisons is to expect near-identical RGB but allow differing alpha as the differences are primarily due to antialiasing. This looks like a non-trivial addition to the testing code.

The threshold is to deal with browsers' issues on test-by-test basis (see it.allowing). Otherwise we use pixel perfect comparisons, because that's what worked the best so far, in particular giving the best robustness of our tests. I experimented with various approaches to allow some color/alpha/luminance variance (see diff_image() in devtools/image.ts, though this uses HSL color model instead of RGB), but all of those approaches were either insufficient or hid real bugs. Though this was done for cross-platform differences, so maybe cross-backend testing would be easier to figure out. In any case, test robustness is the most important thing, even if it means we can't compare canvas, SVG and GL images directly. I don't expect images produced by different backends to be directly comparable, and especially by our automation. Though it would be nice to have, if possible.

[mattpap]

@ianthomas23, let's start issues for the remaining discussions/tasks/bugs.

[ianthomas23]

@mattpap Thanks for the pointers about the image testing. This sort of information is really useful as I find my way around the codebase. I'll write up an issue for the cross-backend testing. Maybe a pragmatic approach is best here of setting up some comparisons as soon as possible, even if they are crude and/or with a high tolerance, and then honing them to make them more useful in time. Although probably lower priority work than some of the other missing things.

My concern is 1 pixel translation errors and suchlike, which the human eye can easily miss (well mine certainly can) so a repeatable programmatic way of checking this would be useful.

[bryevdv]

Thank you @ianthomas23 for jumping in and taking such an involved interest in this, I am really excited for the new state and trajectory of webgl support! 🙏

[bryevdv]

@ianthomas23 Just FYI for the follow-on issues, the examples in the docs WebGL section have some artifacting:

Edit: for easier ref these are the "10k" examples under examples/webgl

[ianthomas23]

@ianthomas23 Just FYI for the follow-on issues, the examples in the docs WebGL section have some artifacting:

Yuck, that is ugly! It looks like incorrect alpha-blending that doesn't occur on my dev system. Presumably there are slightly different defaults on different hardware/browser that I need to explicitly set.